Skip to content

Conversation

@hero78119
Copy link
Collaborator

@hero78119 hero78119 commented Jun 5, 2025

This PR build on top of #799 with one extra 48ded1a to introduce backend expression and cached in constrain system. This align the design with pre-compile so its easier for next step refactor to introduce precompile chip in main flow.
Main sumcheck read/write lookup expression was simplified, as post evaluate() was also removed.

Expression

Expression will be simplified into 2 kind: frontend and backend expression

  • frontend expression: expression with Witin/StructuralWitin/Fixed, in recursive/nested style
  • backend expression: expression with Witin only, in monomial style.

After circuit setup, both expression content are all known and freezed. During runtime, we can take backend expression and evaluate its scalar with "challenge/instance" then the final expression can be put into sumcheck.

benchmark

The nice thing is before/after change, there is no performance difference.

Benchmark Median Time (s) Median Change (%)
fibonacci_max_steps_1048576 2.0641 -0.9869% (No change in performance detected)
fibonacci_max_steps_2097152 3.5514 -1.0748% (Change within noise threshold)
fibonacci_max_steps_1048576 2.0641 -0.9869% (No change in performance detected)

@hero78119 hero78119 requested review from kunxian-xia and spherel June 5, 2025 03:53
@hero78119 hero78119 force-pushed the feat/unify_main_sumcheck branch 2 times, most recently from b33203f to ca8d4e1 Compare June 5, 2025 05:43
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 force-pushed the feat/unify_main_sumcheck branch 2 times, most recently from 1721f07 to 40379b3 Compare June 6, 2025 22:03
@dreamATD dreamATD force-pushed the feat/unify_main_sumcheck branch from 40379b3 to 627a3c6 Compare June 11, 2025 09:50
@dreamATD dreamATD force-pushed the feat/unify_main_sumcheck branch from 627a3c6 to b3472db Compare June 11, 2025 11:47
@dreamATD
Copy link
Collaborator

The newest benchmark compared with the master branch:

Benchmark Median Change (%)
fibonacci_max_steps_1048576 -2.9942%
fibonacci_max_steps_2097152 +1.7704%
fibonacci_max_steps_4194304 +1.9446%

@hero78119
Copy link
Collaborator Author

👍 I tried to run with latest master

Benchmark Median Time (s) Median Change (%)
fibonacci_max_steps_1048576 2.1374 +0.0815% (No change in performance detected)
fibonacci_max_steps_2097152 3.6700 +2.3474% (Change within noise threshold)
fibonacci_max_steps_4194304 6.6210 +1.1196% (Change within noise threshold)

Before/after result looks ok, and overall within noise threshold

@spherel spherel added this pull request to the merge queue Jun 17, 2025
Merged via the queue into scroll-tech:master with commit f39781d Jun 17, 2025
4 checks passed
yczhangsjtu pushed a commit that referenced this pull request Jun 19, 2025
This PR build on top of #799 with one extra
48ded1a
to introduce backend expression and cached in constrain system. This
align the design with pre-compile so its easier for next step refactor
to introduce precompile chip in main flow.
Main sumcheck read/write lookup expression was simplified, as post
`evaluate()` was also removed.

### Expression

Expression will be simplified into 2 kind: frontend and backend
expression
- frontend expression: expression with Witin/StructuralWitin/Fixed, in
recursive/nested style
- backend expression: expression with Witin only, in monomial style.

After circuit setup, both expression content are all known and freezed.
During runtime, we can take backend expression and evaluate its scalar
with "challenge/instance" then the final expression can be put into
sumcheck.


### benchmark

The nice thing is before/after change, there is no performance
difference.

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

|----------------------------------|------------------|----------------------------------------|
| fibonacci_max_steps_1048576 | 2.0641 | -0.9869% (No change in
performance detected) |
| fibonacci_max_steps_2097152 | 3.5514 | -1.0748% (Change within noise
threshold) |
| fibonacci_max_steps_1048576 | 2.0641 | -0.9869% (No change in
performance detected) |
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.

3 participants