Skip to content
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

Nova forward ports #276

Merged
merged 4 commits into from
Jan 22, 2024
Merged

Conversation

huitseeker
Copy link
Member

@huitseeker huitseeker commented Jan 20, 2024

This ports the following upstream PRs:

The next-to-last one should impact our upstream Supernova PR in minor ways, please review.

srinathsetty and others added 3 commits January 20, 2024 12:51
* add bitwise AND example

* fix cargo clippy
* fix typo

* fix typo

* Fix typo
* Remove absorbing of running instance

* Update value of NUM_FE_FOR_RO

* Update expected values in tests

* Add comment

* cargo fmt
@adr1anh
Copy link
Contributor

adr1anh commented Jan 22, 2024

Since SuperNova calls the existing NIFS fold function, we shouldn't have to make any modification to the augmentation circuit, and should be getting these benefits "for free"

@huitseeker
Copy link
Member Author

Since SuperNova calls the existing NIFS fold function, we shouldn't have to make any modification to the augmentation circuit, and should be getting these benefits "for free"

I'm not sure whether your remark is in the context of analyzing the present PR, or about the impact on our Supernova PR. I'll answer each

In the context of the present PR:

In theory, yes, the changes from Nova should be one trivial rebase from fitting into our repo. In practice, we have a separate set of non-allocating functions which are not upstream, that are mostly exemplified by microsoft/Nova#277, and so the code locations modified by Nova upstream need to have equivalent code locations modified in these non-alloc functions (e.g. our nifs/prove_mut)

Further, the changes of the upstream PR have impact on some SuperNova tests, such as constraint counts in the circuit. These SuperNova tests are simply not in the upstream repo, so we just have to compute (through expect_test) the new values.

In the context of impacting the upstream PR:

The upstream Supernova PR introduces tests that are impacted by the RO changes of the next-to-last commit, e.g. constraint counts for the SuperNova circuit, which are not upstream. The introduced material will need to be changed upon rebasing the PR.

huitseeker pushed a commit to huitseeker/arecibo that referenced this pull request Jan 22, 2024
@huitseeker huitseeker added this pull request to the merge queue Jan 22, 2024
Merged via the queue into argumentcomputer:dev with commit d6f4636 Jan 22, 2024
9 checks passed
@huitseeker huitseeker deleted the nova_forward_ports_2 branch January 22, 2024 16:57
gabriel-barrett pushed a commit to gabriel-barrett/arecibo that referenced this pull request Feb 9, 2024
* fix some typos (argumentcomputer#251)

* update readme (argumentcomputer#276)

* Nit: Simplified Vector Initialization (argumentcomputer#278)

* Remove an unnecessary clone in PolyEvalWitness

* Enhance code efficiency by direct vector initialization

---------

Co-authored-by: xiaolou86 <[email protected]>
Co-authored-by: Srinath Setty <[email protected]>
Co-authored-by: Junhee Lee <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants