Skip to content

Conversation

@hero78119
Copy link
Collaborator

@hero78119 hero78119 commented May 27, 2025

Extracted from #952.

Observe a bottleneck on previous interpolation which contribute to most of time due to vector.extend operation and bunch of allocations.
This PR rewrite univariate extrapolation

  1. as the point to be interpolate are fixed set, we can pre-compute all stuff require field inverse
  2. in-place change to avoid allocation

benchmark

In Ceno opcode main sumcheck part we batch different degree > 1 into one batch so this function will be used.
It shows a slightly improvement (~3%) on Fibonacci 2^24 e2e

Benchmark Median Time (s) Median Change (%)
fibonacci_max_steps_1048576 2.3978 +0.9805% (No significant change )
fibonacci_max_steps_2097152 4.2579 +1.7587% (Change within noise)
fibonacci_max_steps_4194304 7.7561 -3.5338%

@hero78119 hero78119 force-pushed the feat/opt_serial_sumcheck branch from 653bac9 to 7b075ec Compare May 27, 2025 03:04
@hero78119 hero78119 requested a review from spherel May 27, 2025 03:14
@hero78119 hero78119 force-pushed the feat/opt_serial_sumcheck branch from 7b075ec to 80fc89f Compare May 27, 2025 15:38
@hero78119 hero78119 changed the title optimize sequential extrapolation optimize extrapolation with zero field inverse during runtime May 27, 2025
@hero78119 hero78119 force-pushed the feat/opt_serial_sumcheck branch from 80fc89f to 6e1ab56 Compare May 27, 2025 15:40
@hero78119 hero78119 force-pushed the feat/opt_serial_sumcheck branch from 6e1ab56 to c0d01d3 Compare May 28, 2025 01:32
))]
pub struct IOPProof<E: ExtensionField> {
pub point: Vec<E>,
pub point: Point<E>,
Copy link
Member

Choose a reason for hiding this comment

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

I just found Point<E> shouldn't be included in IOPProof<E>. What do you think about it?

Copy link
Collaborator Author

@hero78119 hero78119 May 29, 2025

Choose a reason for hiding this comment

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

yes I agree with you we should remove it and I will submit another PR to do that since in involved more change outside of this scope

Copy link
Member

@spherel spherel left a comment

Choose a reason for hiding this comment

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

LGTM!

@hero78119 hero78119 added this pull request to the merge queue May 29, 2025
Merged via the queue into scroll-tech:master with commit 7fc1519 May 29, 2025
4 checks passed
@hero78119 hero78119 deleted the feat/opt_serial_sumcheck branch May 29, 2025 07:03
github-merge-queue bot pushed a commit that referenced this pull request May 29, 2025
build on top of #956 to address review comments
clean up point from sumcheck proof, as verifier should derived itself
hero78119 added a commit to hero78119/ceno that referenced this pull request May 30, 2025
…-tech#956)

Extracted from scroll-tech#952.

Observe a bottleneck on previous interpolation which contribute to most
of time due to `vector.extend` operation and bunch of allocations.
This PR rewrite univariate extrapolation
1. as the point to be interpolate are fixed set, we can pre-compute all
stuff require field inverse
2. in-place change to avoid allocation

In Ceno opcode main sumcheck part we batch different degree > 1 into one
batch so this function will be used.
It shows a slightly improvement (~3%) on Fibonacci 2^24 e2e

| Benchmark | Median Time (s) | Median Change (%) |

|----------------------------------|------------------|--------------------|
| fibonacci_max_steps_1048576 | 2.3978 | +0.9805% (No significant change
) |
| fibonacci_max_steps_2097152 | 4.2579 | +1.7587% (Change within noise)
|
| fibonacci_max_steps_4194304 | 7.7561 | -3.5338% |
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