improve: apply tachyon optimizations(1) #342

guorong009 · 2024-05-29T12:45:51Z

Description

Analyze and apply the tachyon optimizations - 1(Lazy IFFT)

Related issues

Analyze and select optimizations to port from C++ port of Halo2 by kroma-network/tachyon #293

Changes

remove permutation_product_coset from (permutation::)Committed
remove (permutation::)Constructed & use Committed directly

codecov-commenter · 2024-05-29T12:49:42Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.89%. Comparing base (e931d58) to head (4628c24).
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #342      +/-   ##
==========================================
- Coverage   81.90%   81.89%   -0.01%     
==========================================
  Files          82       82              
  Lines       17017    17019       +2     
==========================================
  Hits        13937    13937              
- Misses       3080     3082       +2

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

halo2_backend/src/plonk/permutation/prover.rs

halo2_backend/src/plonk/vanishing/prover.rs

ed255 · 2024-05-30T14:44:53Z

halo2_backend/src/plonk/evaluation.rs

@@ -447,12 +454,13 @@ impl<C: CurveAffine> Evaluator<C> {
                        // - z_i(X) \prod_j (p(X) + \delta^j \beta X + \gamma)
                        // )
                        let mut current_delta = delta_start * beta_term;
-                        for ((set, columns), cosets) in sets
+                        for (((set_idx, _), columns), cosets) in sets


How about iterating over permutation_product_cosets so that we don't need to manually index at each iteration? Something like this:

let mut current_delta = delta_start * beta_term; for ((permutation_product_coset, columns), cosets) in permutation_product_cosets .iter() .zip(p.columns.chunks(chunk_len)) .zip(pk.permutation.cosets.chunks(chunk_len)) { let mut left = permutation_product_coset[r_next];

I think the code looks slightly more clear this way.

davidnevadoc

Good job!
Left a comment/question about a couple of permutation related structs that may be now obsolete.

davidnevadoc · 2024-05-30T14:53:55Z

halo2_backend/src/plonk/permutation/prover.rs

@@ -25,7 +25,6 @@ use halo2_middleware::poly::Rotation;

 pub(crate) struct CommittedSet<C: CurveAffine> {
    pub(crate) permutation_product_poly: Polynomial<C::Scalar, Coeff>,
-    pub(crate) permutation_product_coset: Polynomial<C::Scalar, ExtendedLagrangeCoeff>,


After removing this, how is CommittedSet different from ConstructedSet?
This part of the code is quite poorly documented so it's a bit hard to follow the function of each struct...

The function construct() for Committed may be simplified now to a simple clone() (not sure, some types may need to be fixed).
In fact, after this change I think we should re-evaluate the need of Commited and Constructed, as these may be redundant.

Yes, I agree with you.

How about this update? 4628c24
I am sure that we don't need Constructed struct any more.
It also removes unnecessary conversion, which needs .clone().

davidnevadoc

The optimization looks good!
I still don't understand 100% the logic and purpose behind all the structs surrounding the permutation. I'll try to understand and document that part of the code so it is more understandable.

adria0

Some nitpicks, I do not see any wrong here.
I also tried to check with #345 and it's ok.
Thanks for porting it!

halo2_backend/src/plonk/vanishing/prover.rs

adria0 · 2024-06-03T11:25:13Z

halo2_backend/src/plonk/permutation/prover.rs

 pub(crate) struct Evaluated<C: CurveAffine> {
-    constructed: Constructed<C>,
+    constructed: Committed<C>,


Suggested change

constructed: Committed<C>,

commited: Committed<C>,

Thanks for suggestion!
Here, I believe it is better to use constructed.
One reason is that it is also the constructed in lookup::prover::Evaluated & shuffle::prover::Evaluated.

ed255

LGTM!

privacy-scaling-explorations#342

* Remove book and halo2. Move everything to parent repo. * Remove Shuffle * Remove IPA * Remove SHPLONK * Remove compressed selectors * Temporarily remove examples and benches * Working version of new polynomial commitment scheme interface. * Plonk working parametrised only by F and PCS. * Bring examples and benches back * Update to halo2curves 0.7.0 * Apply tachyon optimizations privacy-scaling-explorations#342 * Cost model update * * Implement PartialEq, Eq, Hash for Cell and AssignedCell * Add table, compressed and normal rows count. * Add rows and table rows to cost model. * Ignore unassigned cells if they are multiplied by zero * Some format values are written as "Scalar(0x..)" The hotfix was to change the stripping rules, but this is probably an incorrect implementation of certain traits for one of the curves. * Review comments * Just moving things around! Used the same style as we have in midnight-circuits. Modules are defined with a `mod.rs` file inside the folder, instead of with a file at root level with the same name. I've also created a module for utils and one for the transcript. * Remove issue template * Review comments * Bring back using the SerdeFormat for keys * Address review comments * Leftover docs

guorong009 added 5 commits May 24, 2024 11:24

feat: remove "permutation_product_coset" from "Committed"

5af7ac4

Merge branch 'main' into gr@tachyon-optimize-1

efbfcac

fix: update "h_commitments" part in "vanishing/prover.rs"

630fe7d

Merge branch 'main' into gr@tachyon-optimize-1

43093a1

Merge branch 'main' into gr@tachyon-optimize-1

e26dfef

guorong009 requested review from adria0, davidnevadoc and ed255 May 29, 2024 12:46

guorong009 self-assigned this May 29, 2024

ed255 reviewed May 30, 2024

View reviewed changes

davidnevadoc reviewed May 30, 2024

View reviewed changes

guorong009 added 2 commits May 31, 2024 10:33

chore: improve iteration

c102941

feat: remove the "Constructed" from permutation proving

4628c24

guorong009 requested review from davidnevadoc and ed255 June 3, 2024 01:59

davidnevadoc approved these changes Jun 3, 2024

View reviewed changes

adria0 approved these changes Jun 3, 2024

View reviewed changes

ed255 approved these changes Jun 3, 2024

View reviewed changes

guorong009 merged commit b4d1c4c into main Jun 3, 2024
18 checks passed

guorong009 deleted the gr@tachyon-optimize-1 branch June 3, 2024 13:07

iquerejeta added a commit to input-output-hk/halo2 that referenced this pull request Dec 12, 2024

Apply tachyon optimizations

961e7d2

privacy-scaling-explorations#342

davidnevadoc mentioned this pull request Dec 19, 2024

Analyze and select optimizations to port from C++ port of Halo2 by kroma-network/tachyon #293

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

improve: apply tachyon optimizations(1) #342

improve: apply tachyon optimizations(1) #342

guorong009 commented May 29, 2024 •

edited

Loading

codecov-commenter commented May 29, 2024 •

edited

Loading

ed255 May 30, 2024

davidnevadoc left a comment

davidnevadoc May 30, 2024

davidnevadoc May 30, 2024

guorong009 May 31, 2024

guorong009 May 31, 2024

davidnevadoc left a comment

adria0 left a comment

adria0 Jun 3, 2024

guorong009 Jun 3, 2024

ed255 left a comment

improve: apply tachyon optimizations(1) #342

improve: apply tachyon optimizations(1) #342

Conversation

guorong009 commented May 29, 2024 • edited Loading

Description

Related issues

Changes

codecov-commenter commented May 29, 2024 • edited Loading

Codecov Report

ed255 May 30, 2024

Choose a reason for hiding this comment

davidnevadoc left a comment

Choose a reason for hiding this comment

davidnevadoc May 30, 2024

Choose a reason for hiding this comment

davidnevadoc May 30, 2024

Choose a reason for hiding this comment

guorong009 May 31, 2024

Choose a reason for hiding this comment

guorong009 May 31, 2024

Choose a reason for hiding this comment

davidnevadoc left a comment

Choose a reason for hiding this comment

adria0 left a comment

Choose a reason for hiding this comment

adria0 Jun 3, 2024

Choose a reason for hiding this comment

guorong009 Jun 3, 2024

Choose a reason for hiding this comment

ed255 left a comment

Choose a reason for hiding this comment

guorong009 commented May 29, 2024 •

edited

Loading

codecov-commenter commented May 29, 2024 •

edited

Loading