577 new fast tbl #611
577 new fast tbl #611
Conversation
CblPOK-git
force-pushed
the
577-new-fast-tbl
branch
2 times, most recently
from
10af322
to
8521548
Compare
CblPOK-git
force-pushed
the
577-new-fast-tbl
branch
7 times, most recently
from
c5bbcfe
to
c6d39d3
Compare
…file in fast gen mode
CblPOK-git
force-pushed
the
577-new-fast-tbl
branch
from
c6d39d3
to
c480e5d
Compare
| nil::blueprint::assigner<BlueprintFieldType> &assigner_instance, | ||
| const std::size_t &ComponentConstantColumns, | ||
| const std::size_t &ComponentSelectorColumns | ||
| nil::blueprint::assigner<BlueprintFieldType> &assigner_instance |
There was a problem hiding this comment.
why not
const std::vector<assignment_proxy<ArithmetizationType>>& assignments
There was a problem hiding this comment.
Why should be? Earlier it was assigner_instance too, just implemented needed feature with minimal change in the existing code
There was a problem hiding this comment.
i guess it good time for fix this palace, or using const here leads problems?
examples/CMakeLists.txt
Outdated
| @@ -62,6 +67,7 @@ function(assign_ir) | |||
| -i ${INPUTS_DIR}/${input} | |||
| ${minus_p} ${private_input_string} | |||
| -c circuit_${target}.crct | |||
| -j table_pieces_${target}.json | |||
There was a problem hiding this comment.
why need table_pieces for generate both?
There was a problem hiding this comment.
it includes generate_crct, and generate_crct generates table_pieces
There was a problem hiding this comment.
we should allow skip table_pieces command parameter if used generate_both
There was a problem hiding this comment.
I don't like that idea, generate_both includes circuit and table generation, first one generates table_pieces. One would expect that circuit generation generates table pieces regardless of whether circuit and table are generated simultaneously or separately. contradicts the principle of least astonishment. And maybe there shouldn't be such thing as table_pieces_name at all. Just use table_pieces_circuit_file_name.crct. What do you think?
| @@ -849,24 +1220,41 @@ int main(int argc, char *argv[]) { | |||
| public_input_file_name = vm["public-input"].as<std::string>(); | |||
| } | |||
|
|
|||
| if (gen_mode.has_assignments()) { | |||
| if (gen_mode.has_circuit() || gen_mode.has_fast_tbl() || gen_mode.has_slow_tbl()) { | |||
There was a problem hiding this comment.
We should not ask assignment-table file name for pure circuit generation mode (even if table somehow generated for calculate used rows)
There was a problem hiding this comment.
Pure circuit generation mode generates constants and selectors part of the assignment table so assignment table file name is needed. Other question is that it may be more reasonable to use circuit file name for constants and selectors, like that:
circuit_file_name.crct
selectors_circuit_file_name.crct
table_header_circuit_file_name.crct
constants_circuit_file_name.crct
witness_assignment_file_name.tbl
pub_inp_assignment_file_name.tbl
I think I will do in that way if you don't mind
Works fine except last commit, revert it and everything will work fine. Is compatible with old version of crypto3 that is not monorep yet and consists of 15 libraries, so please don't delete these libraries.
Possible assigner modes:
circuit: generate crct, table_pieces, table header, constants and selectors using IR.
assignment: generate witness and public_input using IR.
assignment-fast: generate witness and public_input using table pieces and table_header + constants
circuit-assignment: generate everything using IR.
public-input-column: generate public_input using IR.
size_estimation: count components amount using IR