-
-
Notifications
You must be signed in to change notification settings - Fork 44
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
MapToScalarField()
added for Banderwagon points
#278
Merged
Merged
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
advaita-saha
changed the title
Sep 24, 2023
MapToScalarField
added for Banderwagon pointsMapToScalarField()
added for Banderwagon points
mratsim
reviewed
Sep 26, 2023
mratsim
approved these changes
Oct 4, 2023
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.
OK overall,
I would change the array
input to use openArray instead because:
- This is more flexible, many times the input size isn't known at compile-time
- Code size, this creates a proc per
N
(monomorphization), which isn't worth it for such a slow code.
batchAffine has an example on how to do this properly:
- tag the function
{.noInline.}
so that the allocStack doesn't lead to stack overflow if the function is called in a loop and inlined for some reason (LTO ...) - if copy from a temporary, use for loop, it will be optimized by the compiler. (alternatively copyMem https://nim-lang.org/docs/system.html#copyMem%2Cpointer%2Cpointer%2CNatural )
I have also added the tests to nimble file |
Perfect, thanks! |
mratsim
added a commit
that referenced
this pull request
Jun 10, 2024
mratsim
added a commit
that referenced
this pull request
Jun 11, 2024
* PCS: Add vector Pedersen commitments * IPA: unfortunately FR[Banderwagon] does not have a high-enough 2-adicity for optimized Lagrange polynomial * polynomials: rename PolyDomainEval to PolyRootsDomainEval as it's specialized to roots of unity domain * polynomials: introduce formal derivatives and vanishing polynomials * CRS / protocol setups: rename truted_setups folder to prepare for transparent IPA setup * fix: batch inversion zero edge cases, introduced in #278 * feat: refactor and add new lagrange polynomial primitives * refactor(eth-verkle-ipa): delete barycentric_form and use polynomials impl as IPA backend
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.
Fixes #277
Refere the issue #277 for the detailed specs
mapToBaseField()
function for computingMapToScalarField()
for converting themapToBaseField()
&MapToScalarField()
BatchMapToField()
, this does the same operations but for a batch of Banderwagon points together