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

Explicitly only support 1 auxiliary segment #267

Merged
merged 30 commits into from
Apr 3, 2024

Conversation

plafer
Copy link
Contributor

@plafer plafer commented Mar 27, 2024

Removes the half-support for multiple auxiliary segment. We now explicitly support only one.

@plafer plafer marked this pull request as ready for review March 27, 2024 10:39
Copy link
Collaborator

@irakliyk irakliyk left a comment

Choose a reason for hiding this comment

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

Thank you! Looks good! I left some comments inline - most are pretty minor.

air/src/air/coefficients.rs Outdated Show resolved Hide resolved
air/src/air/trace_info.rs Outdated Show resolved Hide resolved
prover/src/lib.rs Show resolved Hide resolved
prover/src/trace/trace_lde/default/mod.rs Outdated Show resolved Hide resolved
prover/src/trace/poly_table.rs Outdated Show resolved Hide resolved
prover/src/trace/poly_table.rs Outdated Show resolved Hide resolved
@plafer plafer requested a review from irakliyk April 2, 2024 20:32
Copy link
Collaborator

@irakliyk irakliyk left a comment

Choose a reason for hiding this comment

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

Looks good! Thank you! I left a coupe of minor comments inline. After these are addressed, we can merge.

There is also a bigger comment about renaming. Currently we use main_segment and main_trace as well as aux_segment and aux_trace somewhat interchangeably. I think we should just use main_trace and aux_trace everywhere. But this renaming should probably be done in another PR.

prover/src/trace/mod.rs Outdated Show resolved Hide resolved
prover/src/constraints/evaluator/default.rs Show resolved Hide resolved
prover/src/trace/poly_table.rs Outdated Show resolved Hide resolved
prover/src/trace/trace_lde/default/mod.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@irakliyk irakliyk left a comment

Choose a reason for hiding this comment

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

All looks good! Thank you!

@irakliyk irakliyk merged commit 8c405b2 into facebook:next Apr 3, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants