Skip to content

Conversation

@dannywillems
Copy link
Member

@dannywillems dannywillems commented Oct 4, 2024

It is more meaningful because it is actually chunks. In following pull requests, the abstraction over PolyComm will be changed, to avoid erroneous usage of PolyComm.

It is part of the implementation of the prover into pickles, as I noticed I made a small error while splitting the quotient polynomial in chunks.

I am breaking the usage of PolyComm in develop, and in Mina, cc @volhovm as you will work on merging develop into master for proof-systems.

@dannywillems dannywillems requested a review from querolita October 4, 2024 12:42
@dannywillems dannywillems changed the title koly-commitment: rename elems field into chunks Poly-commitment: rename elems field into chunks Oct 4, 2024
It is more meaningful because it is actually chunks.
In following pull requests, the abstraction over PolyComm will be changed, to
avoid errorneous usage of PolyComm.
@dannywillems dannywillems force-pushed the dw/poly-comm-rename-elems-in-chunks branch from 86aeee7 to 7f9f96c Compare October 4, 2024 13:10
@codecov
Copy link

codecov bot commented Oct 4, 2024

Codecov Report

Attention: Patch coverage is 85.81081% with 21 lines in your changes missing coverage. Please review.

Project coverage is 72.98%. Comparing base (7751b57) to head (7f9f96c).
Report is 25 commits behind head on master.

Files with missing lines Patch % Lines
poly-commitment/src/commitment.rs 81.63% 9 Missing ⚠️
ivc/src/plonkish_lang.rs 0.00% 3 Missing ⚠️
ivc/src/prover.rs 0.00% 3 Missing ⚠️
poly-commitment/src/ipa.rs 87.50% 2 Missing ⚠️
arrabiata/src/witness.rs 87.50% 1 Missing ⚠️
folding/src/lib.rs 90.90% 1 Missing ⚠️
kimchi/src/verifier.rs 50.00% 1 Missing ⚠️
o1vm/src/legacy/trace.rs 0.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2658   +/-   ##
=======================================
  Coverage   72.98%   72.98%           
=======================================
  Files         244      244           
  Lines       57768    57777    +9     
=======================================
+ Hits        42162    42170    +8     
- Misses      15606    15607    +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@querolita querolita left a comment

Choose a reason for hiding this comment

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

Approving, but let's make sure the mina side is not broken if trying to access elems

@dannywillems
Copy link
Member Author

Approving, but let's make sure the mina side is not broken if trying to access elems

It will be broken.

@dannywillems
Copy link
Member Author

Approving, but let's make sure the mina side is not broken if trying to access elems

It will be broken.

I've checked to backport in develop by cherry-picking, and update Mina accordingly, but proof-systems develop/master are unsynced. I think @volhovm is working on this.

@dannywillems dannywillems merged commit d8a1f6b into master Oct 4, 2024
7 checks passed
@dannywillems dannywillems deleted the dw/poly-comm-rename-elems-in-chunks branch October 4, 2024 14:25
@dannywillems
Copy link
Member Author

I'm taking the responsibility to introduce this breaking change. If you have difficulties when merging develop into master @volhovm, I think it is fair for me to help you to propagate this change in mina + o1js.

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.

3 participants