-
Notifications
You must be signed in to change notification settings - Fork 94
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
[Experiment] Batch inversions of registers accesses done before folding #2047
base: master
Are you sure you want to change the base?
Conversation
This reverts commit ab9359a.
Idea: not using batch_inverse from arkworks, but implementing the batch inversion ourself, ignoring zero
|
Idea from Matthew after our 1:1 to avoid costly u64->Fp transformations: Perhaps we don't need the Montgomery representation until the very end of the folding, and in between we can instead work with BigUint. Even compute the batch inverses as a BigUint modulo the field size. |
I made this a draft because performance is still not as fast as desired (in my machine, Matthew's idea runs at 1.3 Mhz, but this one is about 5 times slower). I believe it is the castings from u64 to F what is causing it. Will go back to it in May after finishing the main goal for part 1. |
@dannywillems actually the |
This is an exploratory PR trying to fix #2040 , where inverses involved in register accesses were substituted with a dummy term (making the constraints unsatisfiable).
Here, I am proposing a new kind of
Column
calledInverseState
with at most 9 elements which will keep track of the values that were supposed to be inverted throughout the step execution. Instead of doing this per-step, we are batching these inversions at the end, before folding the witness. At that time, the witness gets mapped fromu64
's to actualFp
elements, and the columns corresponding to inverses are mapped to their inverse value in the field with a single batch inversion for the whole 9 columns.Given that most instructions won't use the 9 columns for inversions, these columns are set to 1 by default (reason why when we reset the
self.inverse_state
after every step, we initialize them with 1's instead of 0's as for the scratch state).This is just an idea, so please feel free to point out any possible detail I might be missing. I am pointing this out because at first I thought it would be a very complicated change, but the diff right now is not that huge (so I am afraid I might be forgetting about something important).