-
Notifications
You must be signed in to change notification settings - Fork 390
Make aggregation circuit generic over number of snarks #1225
Conversation
@@ -130,6 +130,7 @@ impl RlcConfig { | |||
} | |||
|
|||
#[inline] | |||
#[allow(dead_code)] |
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.
I will make a separate PR that just removes all the dead code outright.
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.
c033047
to
a019641
Compare
i feel it may not be necessary to use constant generic. Anyway not bad. Since it is already written, we can use it. // oh useful for testing? |
num_advice: vec![75], | ||
num_lookup_advice: vec![10], | ||
num_advice: vec![100], | ||
num_lookup_advice: vec![13], |
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.
if we still use chunk num = 15, we don't need to increase nums here?
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.
I am looking into what changes caused the increased column usage. The params were set to their previous values here: increased edb0ae7
Yes, it is mainly for testing. The other option I considered was to use feature flags to vary MAX_AGG_SNARKS, but I thought that that would be harder to maintain. |
Seems you have to increase columns a bit. Try increase as small as possible |
c4d4743
to
5f036b0
Compare
The existing change by zhenfei is the minimum necessary. |
Description
[PR description]
Issue Link
[link issue here]
Type of change
Contents
Rationale
[design decisions and extended information]
How Has This Been Tested?
[explanation]
How to fill a PR description
Please give a concise description of your PR.
The target readers could be future developers, reviewers, and auditors. By reading your description, they should easily understand the changes proposed in this pull request.
MUST: Reference the issue to resolve
Single responsability
Is RECOMMENDED to create single responsibility commits, but not mandatory.
Anyway, you MUST enumerate the changes in a unitary way, e.g.
Design choices
RECOMMENDED to: