Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor commit_inner_layers #986

Merged
merged 1 commit into from
Jan 14, 2025
Merged

Conversation

alon-f
Copy link
Contributor

@alon-f alon-f commented Jan 13, 2025

No description provided.

@reviewable-StarkWare
Copy link

This change is Reviewable

@codecov-commenter
Copy link

codecov-commenter commented Jan 13, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.92%. Comparing base (71516ef) to head (21685f2).
Report is 1 commits behind head on dev.

Additional details and impacted files
@@           Coverage Diff           @@
##              dev     #986   +/-   ##
=======================================
  Coverage   91.91%   91.92%           
=======================================
  Files         105      105           
  Lines       14289    14295    +6     
  Branches    14289    14295    +6     
=======================================
+ Hits        13134    13140    +6     
  Misses       1081     1081           
  Partials       74       74           

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

Copy link
Collaborator

@shaharsamocha7 shaharsamocha7 left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @alon-f and @ilyalesokhin-starkware)


crates/prover/src/core/fri.rs line 197 at r1 (raw file):

    /// Builds and commits to the inner FRI layers (all layers except the first and last).
    ///
    /// All `columns` must be provided in descending order by size.

Can you change the doc so it would be clear that we only have a single column of each size?

Code quote:

/// All `columns` must be provided in descending order by size.

@alon-f alon-f force-pushed the alon/refactor_commit_inner_layers branch from c4d2235 to 4e670fb Compare January 14, 2025 09:21
Copy link
Contributor Author

@alon-f alon-f left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @ilyalesokhin-starkware and @shaharsamocha7)


crates/prover/src/core/fri.rs line 197 at r1 (raw file):

Previously, shaharsamocha7 wrote…

Can you change the doc so it would be clear that we only have a single column of each size?

Done.

Copy link
Collaborator

@shaharsamocha7 shaharsamocha7 left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @ilyalesokhin-starkware)

@alon-f alon-f force-pushed the alon/refactor_commit_inner_layers branch from 4e670fb to 0a1c1cc Compare January 14, 2025 14:01
Copy link
Contributor Author

@alon-f alon-f left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r2.
Reviewable status: 0 of 1 files reviewed, all discussions resolved (waiting on @ilyalesokhin-starkware and @shaharsamocha7)

Copy link
Contributor Author

@alon-f alon-f left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @ilyalesokhin-starkware)

Copy link
Collaborator

@shaharsamocha7 shaharsamocha7 left a comment

Choose a reason for hiding this comment

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

Reviewed all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @ilyalesokhin-starkware)

@alon-f alon-f force-pushed the alon/refactor_commit_inner_layers branch from 0a1c1cc to 21685f2 Compare January 14, 2025 14:04
@alon-f alon-f merged commit 29d124e into dev Jan 14, 2025
15 of 19 checks passed
@alon-f alon-f deleted the alon/refactor_commit_inner_layers branch January 14, 2025 14:25
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.

4 participants