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 MSM with chunks and HashMapPippenger #397

Merged
merged 18 commits into from
Mar 21, 2022

Conversation

huyuncong
Copy link
Member

@huyuncong huyuncong commented Mar 11, 2022

Description

Add steaming multi-scalar multiplication algorithm with chunks.
Add Pippenger algorithm using hash map struct.

Note that we use #![feature(iter_advance_by)] in the MSM with chunks.


Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

  • Targeted PR against correct branch (master)
  • Linked to GitHub issue with discussion and accepted design OR have an explanation in the PR that describes this work.
  • Wrote unit tests
  • Updated relevant documentation in the code
  • Added a relevant changelog entry to the Pending section in CHANGELOG.md
  • Re-reviewed Files changed in the GitHub PR explorer

@huyuncong huyuncong changed the title Add MSM with hard-coded chunk and HashMapPippenger Add MSM with chunk and HashMapPippenger Mar 13, 2022
@huyuncong huyuncong changed the title Add MSM with chunk and HashMapPippenger Add MSM with chunks and HashMapPippenger Mar 13, 2022
@weikengchen
Copy link
Member

It seems that the biggest problem is that we cannot use unstable features in arkworks-rs. And iter_advance_by does not seem to become stable very soon.

Any workaround for now?

.entry(*base.borrow())
.or_insert(G::ScalarField::zero());
*entry += *scalar.borrow();
if self.buffer.len() == self.buffer.capacity() {
Copy link
Member

Choose a reason for hiding this comment

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

@huyuncong @mmaker I have a concern about whether capacity() will run in the expected way. Rust did not promise that the capacity of the map would not grow on its own, and Rust did not promise that it will allocate exactly the capacity as specified during with_capacity.

Note: Rust's hashmap is often constructed through the RawTable here, which may specify a slightly different capacity compared with the one from with_capacity.
https://docs.rs/hashbrown/latest/src/hashbrown/raw/mod.rs.html#191

Avoid relying on the hashmap `capacity()` method, by adding another
field `buf_size` (that echoes `ChunkedPippenger`)
This makes the streaming API for pippenger mkore solid and consistent
with HahMapPippenger.
@mmaker
Copy link
Member

mmaker commented Mar 18, 2022

All pending issues should be resolved now! However, I note that there is no unit-test! @huyuncong can you take care of it?

@Pratyush
Copy link
Member

If you could run cargo fmt that'd be great!

@mmaker
Copy link
Member

mmaker commented Mar 18, 2022

Done! (staging only the changes in ec/msm/)

ec/Cargo.toml Outdated Show resolved Hide resolved
@weikengchen weikengchen merged commit 13c298c into arkworks-rs:master Mar 21, 2022
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.

4 participants