Skip to content

feat: check op queue wires are zero past minicircuit in Translator#16858

Merged
maramihali merged 5 commits intomerge-train/barretenbergfrom
mm/op-queue-check
Sep 10, 2025
Merged

feat: check op queue wires are zero past minicircuit in Translator#16858
maramihali merged 5 commits intomerge-train/barretenbergfrom
mm/op-queue-check

Conversation

@maramihali
Copy link

@maramihali maramihali commented Sep 8, 2025 •
edited
Loading

At each step of the merge protocol we chech that the left_table is of appropriate degree to ensure merging the subtable doesn't cause an overlap. However, as the last subtable is appended so it's the right_table, we don't perform a degree check on it currently. We have to ensure merging the last subtable doesn't cause the number of UltraOps does not go over the fixed Translator MINICIRCUIT_SIZE and the cheapest way to do this is by updating the TranslatorZeroConstraintRelation to ensure the op queue wires are also zero past the MINICIRCUIT_SIZE.

On top of that, this PR reviews and document the skip conditions for the relations found in translator_extra_relations.hpp

Closes AztecProtocol/barretenberg#1399

@maramihali maramihali self-assigned this Sep 8, 2025
ledwards2225
ledwards2225 approved these changes Sep 9, 2025
View reviewed changes
Copy link
Contributor

@ledwards2225 ledwards2225 left a comment

Choose a reason for hiding this comment

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

LG - I'm a little bit confused about the skip conditions though. I think those would benefit from some comments

barretenberg/cpp/src/barretenberg/relations/translator_vm/translator_extra_relations.hpp
{
return (in.lagrange_odd_in_minicircuit + in.lagrange_last_in_minicircuit + in.lagrange_result_row).is_zero();
return (in.lagrange_odd_in_minicircuit + in.lagrange_last_in_minicircuit + in.lagrange_result_row +
in.lagrange_mini_masking)
Copy link
Contributor

@ledwards2225 ledwards2225 Sep 9, 2025

Choose a reason for hiding this comment

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

is this an unrelated fix?

Copy link
Contributor

@ledwards2225 ledwards2225 Sep 9, 2025

Choose a reason for hiding this comment

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

Also, I'm surprised that we want to skip only when NOT masking.. is that intentional?

barretenberg/cpp/src/barretenberg/relations/translator_vm/translator_extra_relations.hpp Outdated
{
static constexpr auto minus_one = -FF(1);
return (in.lagrange_even_in_minicircuit + in.lagrange_last_in_minicircuit + minus_one).is_zero();
return (in.lagrange_even_in_minicircuit + in.lagrange_last_in_minicircuit + in.lagrange_mini_masking +
Copy link
Contributor

@ledwards2225 ledwards2225 Sep 9, 2025

Choose a reason for hiding this comment

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

I'm confused about this change and its hard to read because its not clear to which polynomial we're logically applying the -1

@DanielKotov DanielKotov force-pushed the merge-train/barretenberg branch from 20c4680 to 60bc9ab Compare September 10, 2025 08:15
ledwards2225
ledwards2225 reviewed Sep 10, 2025
View reviewed changes
barretenberg/cpp/src/barretenberg/relations/translator_vm/translator_extra_relations.hpp
// no-op (i.e. op is zero at an even index)
static constexpr auto minus_one = -FF(1);
return (in.lagrange_even_in_minicircuit + in.lagrange_last_in_minicircuit + minus_one).is_zero();
return (in.lagrange_even_in_minicircuit + in.op + minus_one).is_zero() ||
Copy link
Contributor

@ledwards2225 ledwards2225 Sep 10, 2025

Choose a reason for hiding this comment

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

I guess this is ok given that 1 is not a valid value for op (right?)

Copy link
Author

@maramihali maramihali Sep 10, 2025

Choose a reason for hiding this comment

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

Yes, op can be 0,3,4,8

@maramihali maramihali merged commit 33da0be into merge-train/barretenberg Sep 10, 2025
6 checks passed
@maramihali maramihali deleted the mm/op-queue-check branch September 10, 2025 20:28
@maramihali maramihali mentioned this pull request Sep 10, 2025
Closed
@AztecBot AztecBot mentioned this pull request Sep 10, 2025
Merged
github-merge-queue bot pushed a commit that referenced this pull request Sep 11, 2025
@ludamad
feat: merge-train/barretenberg (#16928)
486c3c7 
BEGIN_COMMIT_OVERRIDE
fix: Origin Tags edgecase (#16921)
chore: cycle group cleanup #2 (#16876)
chore: civc tidy 3 (#16671)
refactor(bb): optimize batch_mul_with_endomorphism (#16905)
feat: check op queue wires are zero past minicircuit in Translator
(#16858)
feat: Add CPU scaling benchmark script for remote execution (#16918)
fix: Add free witness tag to field constructor (#16827)
fix(bb): darwin build (#16957)
END_COMMIT_OVERRIDE
Umarb97 pushed a commit to Umarb97/aztec-packages that referenced this pull request Sep 16, 2025
@Umarb97
feat: check op queue wires are zero past minicircuit in Translator (A…
c927efd 
…ztecProtocol#16858)

At each step of the merge protocol we chech that the `left_table` is of
appropriate degree to ensure merging the subtable doesn't cause an
overlap. However, as the last subtable is appended so it's the
`right_table`, we don't perform a degree check on it currently. We have
to ensure merging the last subtable doesn't cause the number of UltraOps
does not go over the fixed Translator `MINICIRCUIT_SIZE` and the
cheapest way to do this is by updating the
`TranslatorZeroConstraintRelation` to ensure the op queue wires are also
zero past the `MINICIRCUIT_SIZE`.

On top of that, this PR reviews and document the `skip` conditions for
the relations found in `translator_extra_relations.hpp`

Closes AztecProtocol/barretenberg#1399
mralj pushed a commit that referenced this pull request Oct 13, 2025
@mralj
feat: check op queue wires are zero past minicircuit in Translator (#…
61ad626 
…16858)

At each step of the merge protocol we chech that the `left_table` is of
appropriate degree to ensure merging the subtable doesn't cause an
overlap. However, as the last subtable is appended so it's the
`right_table`, we don't perform a degree check on it currently. We have
to ensure merging the last subtable doesn't cause the number of UltraOps
does not go over the fixed Translator `MINICIRCUIT_SIZE` and the
cheapest way to do this is by updating the
`TranslatorZeroConstraintRelation` to ensure the op queue wires are also
zero past the `MINICIRCUIT_SIZE`.

On top of that, this PR reviews and document the `skip` conditions for
the relations found in `translator_extra_relations.hpp`

Closes AztecProtocol/barretenberg#1399
ludamad pushed a commit that referenced this pull request Dec 16, 2025
@ludamad
feat: check op queue wires are zero past minicircuit in Translator (#…
97f54ee 
…16858)

At each step of the merge protocol we chech that the `left_table` is of
appropriate degree to ensure merging the subtable doesn't cause an
overlap. However, as the last subtable is appended so it's the
`right_table`, we don't perform a degree check on it currently. We have
to ensure merging the last subtable doesn't cause the number of UltraOps
does not go over the fixed Translator `MINICIRCUIT_SIZE` and the
cheapest way to do this is by updating the
`TranslatorZeroConstraintRelation` to ensure the op queue wires are also
zero past the `MINICIRCUIT_SIZE`.

On top of that, this PR reviews and document the `skip` conditions for
the relations found in `translator_extra_relations.hpp`

Closes AztecProtocol/barretenberg#1399
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ledwards2225 ledwards2225 ledwards2225 approved these changes

Assignees

@maramihali maramihali

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@maramihali @ledwards2225