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

Add NIFSGadgetTrait, implement Mova's NIFSGadget, adapt Nova NIFSGadget into NIFSGadgetTrait #173

Merged
merged 1 commit into from
Nov 12, 2024

Conversation

arnaucube
Copy link
Collaborator

@arnaucube arnaucube commented Oct 27, 2024

  • add new NIFSGadgetTrait

  • refactor Nova's NIFSGadget to fit into the new NIFSGadgetTrait

  • implement Mova's NIFSGadget

  • abstract NIFSGadget related tests for all implementors of
    NIFSGadgetTrait to avoid duplicated code in the tests between the
    different Nova variants gadget tests

  • frontends/noir: update mimc usage since it has been migrated from noir's std into it's own repo.

    • this has been done to fix the GitHub CI error, which was failing (also in the latest main commit) due 4 days ago Noir released a new version which moves the mimc lib from std to a separate repo)

@arnaucube arnaucube force-pushed the nifs-gadget-trait branch 2 times, most recently from 91e81a1 to 4ce5100 Compare October 27, 2024 21:33
@arnaucube arnaucube changed the title Add NIFSGadgetTrait, implement Mova's NIFSGadget, adapt Nova NIFSGadet into NIFSGadgetTrait Add NIFSGadgetTrait, implement Mova's NIFSGadget, adapt Nova NIFSGadget into NIFSGadgetTrait Oct 27, 2024
@arnaucube arnaucube force-pushed the nifs-gadget-trait branch 3 times, most recently from 4116358 to 6431558 Compare October 27, 2024 21:43
@arnaucube arnaucube requested review from CPerezz and dmpierre October 27, 2024 21:49
@CPerezz
Copy link
Member

CPerezz commented Oct 29, 2024

Just finished with #145
Will try to target this tomorrow

@arnaucube arnaucube force-pushed the nifs-gadget-trait branch 8 times, most recently from a38a01a to 27efd41 Compare November 4, 2024 17:51
@arnaucube
Copy link
Collaborator Author

Updated the PR rebasing to latest main branch changes after #145 has been merged.

@arnaucube arnaucube mentioned this pull request Nov 7, 2024
Copy link
Member

@CPerezz CPerezz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!!!

Just some comments. Feel free to dismiss.

folding-schemes/src/folding/nova/nifs/mod.rs Outdated Show resolved Hide resolved
folding-schemes/src/folding/nova/nifs/mod.rs Show resolved Hide resolved
…et into NIFSGadgetTrait

* add new NIFSGadgetTrait

* implement Mova's NIFSGadget

* refactor Nova NIFSGadget to fit into the new NIFSGadgetTrait

* abstract NIFSGadget related tests for all implementors of
  NIFSGadgetTrait to avoid duplicated code in the tests between the
  different Nova variants gadget tests

* frontends/noir update mimc usage since it has been migrated from
  noir's std into it's own repo
@arnaucube arnaucube added this pull request to the merge queue Nov 12, 2024
Merged via the queue into main with commit e118387 Nov 12, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants