Skip to content

Conversation

@hero78119
Copy link
Collaborator

@hero78119 hero78119 commented Mar 4, 2025

This PR take basefold mpcs as pilot to take new mpcs api in preparation for p3 library.

Change scope

  • remove plonky2 dependency from mpcs/poseidon
  • mpcs: remove basecode random code as we only need keep reed solomon encoding as our major use case
  • make everything work in evaluation (lagrange domain) and avoid unnessesary coefficient format conversion
  • mpcs: remove basefold commit with extension field as we dont have this scenario.
  • mpcs: use plonky3 dft + mmcs for basefold batch-commit

proof size

New design also introduce a minor optimisation to reduce 5% proof size : during opening phase, we open 2 value of merkle tree on leafs[j], leafs[j+ni], but one of the value can be carry over from previous layer interpolation result, thus prover can skip one of them. With that, we can make this check happened naturally with smaller size of proof.

  • Fibonacci 2^20 19.07mb -> 17.99mb => 5.66% proof reduction
  • Fibonacci 2^21 20.06mb -> 18.94mb => 5.58% proof reduction
  • Fibonacci 2^22 21.10mb -> 19.94mb => 5.49% proof reduction

e2e Latency

on AMD EPYC 32 cores
Fibonacci 2^20

                        time:   [3.6361 s 3.6584 s 3.6883 s]
                        change: [-8.5478% -7.9507% -7.1226%] (p = 0.00 < 0.05)
                        Performance has improved.

Fibonacci 2^21

fibonacci_max_steps_2097152/prove_fibonacci/fibonacci_max_steps_2097152
                        time:   [5.9022 s 5.9411 s 5.9762 s]
                        change: [-12.483% -11.835% -11.205%] (p = 0.00 < 0.05)
                        Performance has improved.

Fibonacci 2^22

fibonacci_max_steps_4194304/prove_fibonacci/fibonacci_max_steps_4194304
                        time:   [11.157 s 11.189 s 11.216 s]
                        change: [-12.124% -11.493% -10.773%] (p = 0.00 < 0.05)
                        Performance has improved.

on AMD 5800X 16 cores

Fibonacci 2^20

fibonacci_max_steps_1048576/prove_fibonacci/fibonacci_max_steps_1048576
                        time:   [5.9030 s 6.1262 s 6.3665 s]
                        change: [-18.746% -15.770% -12.351%] (p = 0.00 < 0.05)
                        Performance has improved.

Fibonacci 2^21

fibonacci_max_steps_2097152/prove_fibonacci/fibonacci_max_steps_2097152
                        time:   [10.681 s 10.710 s 10.738 s]
                        change: [-21.564% -21.252% -20.958%] (p = 0.00 < 0.05)
                        Performance has improved.

@hero78119 hero78119 marked this pull request as draft March 4, 2025 03:37
@hero78119 hero78119 changed the title WIP mpcs commit & opening code refactor cleanup WIP mpcs (basefold) commit & opening code refactor cleanup Mar 4, 2025
github-merge-queue bot pushed a commit that referenced this pull request Mar 5, 2025
Extracted from #843 
This PR retain nice property and clean up bunch of unnessesary bit
reverse across mpcs basefold prover/verifier.
The core idea is to retain raw message in even-odd fold, which match the
sumcheck implementation parameter binding order.
@hero78119 hero78119 force-pushed the feat/opt_basefold branch from 2596174 to bebe12e Compare March 5, 2025 12:55
@hero78119 hero78119 force-pushed the feat/opt_basefold branch from bebe12e to bd4fd5e Compare March 5, 2025 13:22
@hero78119 hero78119 changed the title WIP basefold commit & opening code refactor cleanup basefold commit & opening code refactor cleanup Mar 21, 2025
@hero78119 hero78119 changed the title basefold commit & opening code refactor cleanup basefold commit & opening refactor & cleanup Mar 21, 2025
Comment on lines 24 to 25
pub struct RSCodeDefaultSpec {}

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should change this to

pub RSCodeParams {
   pub num_queries: usize,
   pub log_blowup: usize,
   pub log_basecode_msg_size: usize,
}

impl RSCodeParams {
   pub fn standard_fast() -> Self {
        standard_fri_params_with_100_bits_conjectured_security(1)
    }

    pub fn standard_with_100_bits_conjectured_security(log_blowup: usize) -> Self {
        standard_fri_params_with_100_bits_conjectured_security(log_blowup)
    }
}

impl Default for RSCodeParams {
   fn default() -> Self {
      Self::standard_fast()
   }
}

The rationale is that we can use different param for different scenarios. For example, the base layer we will log_blowup = 1 for maximal performance, while for the last layer in recursion we use log_blowup = 3 for minimal proof size.

pub fn pcs_commit<E: ExtensionField, Pcs: PolynomialCommitmentScheme<E>>(
pp: &Pcs::ProverParam,
poly: &DenseMultilinearExtension<E>,
rmm: RowMajorMatrix<<E as ExtensionField>::BaseField>,
Copy link
Collaborator

Choose a reason for hiding this comment

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

what's the difference between pcs_commit and pcs_batch_commit?

Copy link
Collaborator

@kunxian-xia kunxian-xia left a comment

Choose a reason for hiding this comment

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

great job!

@kunxian-xia kunxian-xia added this pull request to the merge queue Apr 1, 2025
Merged via the queue into scroll-tech:master with commit e2cb027 Apr 1, 2025
4 checks passed
@hero78119 hero78119 mentioned this pull request Apr 1, 2025
3 tasks
github-merge-queue bot pushed a commit that referenced this pull request Apr 3, 2025
Follow up on #843
Clean up basefold sumcheck and unified devirgo sumcheck with ceno. 
Previously in #653 before/after
change is insignificant, but after bunch of refactor and optimised other
critical path, now the benchmark shows promising result.

### Change highlights
- [x] switch to ceno sumcheck and break down basefold sumcheck into 2
phases
> we can't directly use ceno sumcheck as blackbox because of FRI part in
basefold.
- [x] and use evaluation form as uni-variates
- [x] clean up old sumcheck impl

### benchmark 
With e2e
Fibonacci 2^20

```
fibonacci_max_steps_1048576/prove_fibonacci/fibonacci_max_steps_1048576
                        time:   [3.1789 s 3.2022 s 3.2274 s]
                        change: [-14.457% -13.632% -12.780%] (p = 0.00 < 0.05)
                        Performance has improved.
```

Fibonacci 2^21
```
fibonacci_max_steps_2097152/prove_fibonacci/fibonacci_max_steps_2097152
                        time:   [5.2820 s 5.3010 s 5.3207 s]
                        change: [-16.128% -15.753% -15.306%] (p = 0.00 < 0.05)
                        Performance has improved.
```

Fibonacci 2^22
```
fibonacci_max_steps_4194304/prove_fibonacci/fibonacci_max_steps_4194304
                        time:   [10.369 s 10.397 s 10.423 s]
                        change: [-10.485% -10.041% -9.5790%] (p = 0.00 < 0.05)
                        Performance has improved.
```
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.

2 participants