-
Notifications
You must be signed in to change notification settings - Fork 40
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Express transition constraints as circuits and autogenerate code for its evaluation #117
Merged
sshine
merged 26 commits into
master
from
thv-ssh/generate-transition-codeword-evaluation-code
Nov 15, 2022
Merged
Express transition constraints as circuits and autogenerate code for its evaluation #117
sshine
merged 26 commits into
master
from
thv-ssh/generate-transition-codeword-evaluation-code
Nov 15, 2022
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sword-Smith
added
✨ enhancement
Improvement or new feature
🤖 code
Changes the implementation
labels
Nov 11, 2022
Relates to #11 |
jan-ferdinand
approved these changes
Nov 11, 2022
Sword-Smith
force-pushed
the
thv-ssh/generate-transition-codeword-evaluation-code
branch
from
November 11, 2022 18:41
401bb84
to
2ed22ef
Compare
jan-ferdinand
force-pushed
the
master
branch
from
November 11, 2022 18:50
984a3f2
to
10064b2
Compare
9 tasks
jan-ferdinand
force-pushed
the
master
branch
2 times, most recently
from
November 11, 2022 18:56
7334757
to
bd2cf06
Compare
Sword-Smith
force-pushed
the
thv-ssh/generate-transition-codeword-evaluation-code
branch
from
November 12, 2022 14:31
b7c5baa
to
08757bf
Compare
Comment on lines
+348
to
+350
if let Some(p) = maybe_profiler.as_mut() { | ||
p.start("initial quotients") | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I recommend using the macros prof_start
and prof_stop
. Example use can be seen here.
jan-ferdinand
force-pushed
the
master
branch
from
November 14, 2022 07:18
6aa47c3
to
3ffbf9e
Compare
This commit addresses two optimizations: - Avoid repeated transposing codewords in inner loop - Avoid unnecessary vector allocation in inner loop Before this change, we transpose each table once for each inverse zerofier for each type of quotient, which is FRI domain length times. By injecting the transposed extension table, this transposition is hoisted out of those loops. After this change, two cleanup tasks are left: - Change `Evaluable::evaluate_transition_constraints` to not rely on `*_inner`. - Change `ExtHashTable::evaluate_transition_constraints` to not allocate `evaluation_point`. Co-authored-by: Simon Shine <simon@neptune.cash>
Co-authored-by: Simon Shine <simon@neptune.cash>
- Generalize to work for all tables (except hash table) - Fix how challenges are read, from index to struct fields - Add inline pragma to generated code - Fix minor syntax bugs
…y-first The dependency `twenty-first` get's support for adding, subtracting, and multiplying X field elements with B field elements. This is done to speed up the evaluation of auto-generated code. Mutliplying a B field element with an X field element costs 3 B field multiplications, for two X field elements, it costs 9 B field multiplications.
Prior to this commit, this test used to different sets of challenges for constructing the extension constraints and then later in their evaluation. Since this argument was until now not used in the evaluation (since the challenges were contained in the multivariate polynomials describing the constraints), this problem did not surface. But now that we're autogenerating code to evalaute the constraint polynomials, this problem surfaced and it just cost me almost half a day's work.
With auto-generated code these names need to be consistent. This commit removes an 'S' from some of their enum names.
With this draft, the tests should work without the autogenerated code. They'll just be a lot slower. The compiler for evaluating the constraints will overwrite the content in the `autogen` files when it is run.
Record the time it takes to calculate each quotient: initial, consistency, transition, and terminal.
This is necessary for being able to arbitrarily add, subtract and multiply XFieldElements with BFieldElements.
This ensures that the constraint code generator works for all tables.
…onstraints This commit allows single row constraints to be expressed as a circuit and to work with the compiler for auto-generating efficient constraint evaluating Rust code. This is achieved by changing the input type from holding a `usize` to holding a `InputIndicator` type that can hold either single row and dual row constraints. In the generated code the variable name of an input type is assumed to be the same as the `Display` implementation of the `InputIndicator` types.
Sword-Smith
force-pushed
the
thv-ssh/generate-transition-codeword-evaluation-code
branch
from
November 14, 2022 12:05
2a44cb0
to
4dc3ca4
Compare
Sword-Smith
changed the title
Change
Express transition constraints as circuits and autogenerate code for its evaluation
Nov 14, 2022
*_quotients()
to accept transposed extension table dataCo-authored-by: sword-smith <thor@neptune.cash>
- inline `evaluate_transition_constraints_inner` again - Give up removing `evaluation_point` since `.evaluate()` depends on a slice.
sshine
reviewed
Nov 14, 2022
jan-ferdinand
previously requested changes
Nov 14, 2022
With this commit only visited counters that are present in the multitree are checked for declarations in the compiler. This speeds up the compiler by about a factor 60.
These are the cases that stand out syntactically. RamTable: Name intermediate steps in extend().
- Refactor main(). - Add explanations to `InputIndicator` etc.
This gives access to transpose()
jan-ferdinand
deleted the
thv-ssh/generate-transition-codeword-evaluation-code
branch
November 15, 2022 18:44
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit addresses two optimizations:
Before this change, we transpose each table once for each inverse zerofier for each type of quotient, which is FRI domain length times. By injecting the transposed extension table, this transposition is hoisted out of those loops.
After this change, two cleanup tasks are left:
Evaluable::evaluate_transition_constraints
to not rely on*_inner
.ExtHashTable::evaluate_transition_constraints
to not allocateevaluation_point
.This speeds up the evaluation of the transition quotient for the processor table by x2000. On my Dell XPS machine, the evaluation took 32 seconds before these changes, and 15ms after.
Also: The tests took 270 seconds to run on my fast P17 machine prior to this PR. With this PR it takes ~50 seconds. Maybe we can enable some of the tests that were disabled because they were too slow?
To run the compiler for autogenerating the fast code:
OUTPUT_RUST_SOURCE_CODE=1 cargo run --bin constraint-evaluation-generator
Co-authored-by: Simon Shine simon@neptune.cash