Conversation
9c55f77 to
7a366bd
Compare
|
Out of caution, I would have removed the tagging and its infra last thing. This would have allowed us to guard for any changes that should be caught by OOD evaluation mismatches and make sure that these mismatches are justified. |
0e5fd48 to
74a873a
Compare
a54813a to
0d20707
Compare
Snapshot of the uncompressed constraint-equivalence log across the origin/next merge and the three post-merge refactor commits (periodic column typing, hasher chiplet split, hasher controller lifetime restructure). Committed as a trace before lossless compression of the file for PR comment upload. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…r links Five sections (#14, #16, #17, #37, #38) each repeat a single narrative paragraph verbatim across many "Interpretation" cells in their constraint table. The narrative is already present at the top of each section, so the table cells are pure duplication. Add an inline <a id="cN-cat"></a> anchor to each narrative and replace the duplicate cells with a short link [↑ §N narrative](#cN-cat). Non- duplicate rows (with per-row-specific interpretations) are untouched. File size: 88,666 → 43,803 chars. Lossless — the links resolve to the identical prose. Enables posting the full report as a single GitHub PR comment under the 65,536-char limit. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Constraint Equivalence Report (updated)Supersedes the previous report in the comment above. Generated from Constraint ChangesCumulative record of constraint polynomial changes across the simplification If regenerating after a rebase, re-run the constraint recorder rather than 1. refactor: inline tagged_assert_zero variants, remove tagging indirectionNo constraint changes. (441 base + 24 ext) 2. refactor: inline trivial assert wrappers in stack and decoder modulesNo constraint changes. (441 base + 24 ext) 3. refactor: remove empty section headers and apply lint fixesNo constraint changes. (441 base + 24 ext) 4. refactor: remove constraint tagging infrastructureNo constraint changes. (441 base + 24 ext) 5. refactor: introduce MidenAirBuilder trait aliasNo constraint changes. (441 base + 24 ext) 6. refactor: remove unused decoder constants and inline last assert wrapperNo constraint changes. (441 base + 24 ext) 7. refactor: remove dead code and unnecessary allow(dead_code) annotationsNo constraint changes. (441 base + 24 ext) 8. refactor: remove unnecessary clone() calls on Copy typesNo constraint changes. (441 base + 24 ext) 9. refactor: use semantic assertion methods in constraint codeNo constraint changes. (441 base + 24 ext) 10. refactor: inline field constants and centralize in constants.rsNo constraint changes. (441 base + 24 ext) 11. refactor: introduce BoolNot trait for boolean negation in constraintsNo constraint changes. (441 base + 24 ext) 12. refactor: defer Var-to-Expr conversions and simplify array constructionNo constraint changes. (441 base + 24 ext) 13. refactor: add ChipletFlags/ChipletSelectors structs with precomputed flagsNo constraint changes. (441 base + 24 ext) 14. refactor: thread ChipletSelectors through all constraint and bus functions18 updated | 447 unchanged Identical assertion expressions; fingerprints changed because scoped builder gates were recomputed using precomputed 18 constraint changesUpdated:
15. refactor: enforce all chiplet selectors are 1 in last row5 added | 465 unchanged 5 new last-row constraints enforce chiplet selectors s0–s4 equal 1 at the final trace row ( Added:
16. refactor: remove when_transition from chiplet constraints61 updated | 409 unchanged
61 constraint changesUpdated:
17. refactor: add per-bus domain separation to Challenges encoding8 updated | 462 unchanged
8 constraint changesUpdated:
18. refactor: narrow per-chiplet constraint functions to &ChipletFlagsNo constraint changes. (446 base + 24 ext) 19. refactor: inline small constraint helper functions1 updated | 469 unchanged Helper Updated:
20. refactor: add typed column structs, col map, and Borrow implsNo constraint changes. (446 base + 24 ext) 21. refactor: wire MainCols into eval() — MainTraceRow becomes type aliasNo constraint changes. (446 base + 24 ext) 22. refactor: migrate range constraints to named struct fieldsNo constraint changes. (446 base + 24 ext) 23. refactor: migrate decoder constraints to DecoderCols named fieldsNo constraint changes. (446 base + 24 ext) 24. refactor: migrate stack constraints to StackCols named fieldsNo constraint changes. (446 base + 24 ext) 25. refactor: migrate bitwise chiplet constraints to BitwiseColsNo constraint changes. (446 base + 24 ext) 26. refactor: migrate hasher + memory chiplet constraints to typed colsNo constraint changes. (446 base + 24 ext) 27. refactor: migrate ACE chiplet constraints to AceColsNo constraint changes. (446 base + 24 ext) 28. refactor: migrate kernel ROM constraints to KernelRomColsNo constraint changes. (446 base + 24 ext) 29. refactor: migrate chiplet bus wiring + hash_kernel to typed colsNo constraint changes. (446 base + 24 ext) 30. refactor: migrate chiplets bus constraints to typed column structsNo constraint changes. (446 base + 24 ext) 31. refactor: post-review cleanup of typed column structsNo constraint changes. (446 base + 24 ext) 32. refactor: add typed chiplet accessors to MainCols, eliminate raw chiplets accessNo constraint changes. (446 base + 24 ext) 33. chore: format imports in chiplet constraint filesNo constraint changes. (446 base + 24 ext) 34. refactor: split chiplets bus into requests/responses modules, reduce cloningNo constraint changes. (446 base + 24 ext) 35. refactor: apply constraint style rules — when() decomposition, semantic assertions10 updated | 460 unchanged Sign flip: 10 constraint changesUpdated:
36. refactor: flatten single-file stack constraint modules into sibling filesNo constraint changes. (446 base + 24 ext) 37. refactor: type ACE shared columns into named fields with QuadFeltExpr7 updated | 463 unchanged ACE shared columns restructured from flat 7 constraint changesUpdated:
38. refactor: type periodic columns into named structs and simplify chiplet constraints5 updated | 465 unchanged kernel_rom.rs:70-73→62-65. Identical assertion expressions ( Updated:
39. refactor: simplify op_flags with bit-selector pattern and iterative expansion4 updated | 466 unchanged Updated:
40. refactor: use Algebra trait bound and remove op_flags indirectionNo constraint changes. (446 base + 24 ext) 41. fix: use core::array for no-std compatibilityNo constraint changes. (446 base + 24 ext) 42. refactor(decoder): improve readability of composite flagsNo constraint changes. (446 base + 24 ext) 43. refactor(decoder): inline all constraint helpers into enforce_main and rewrite documentation3 updated | 467 unchanged Updated:
(merge) Merge origin/next into constraint-simplificationMerge pulled in 44. refactor: unify periodic column generation with typed struct constructorsNo constraint changes. (509 base + 26 ext) 45. refactor: split hasher chiplet into controller and permutation sub-modules122 updated | 2 removed | 4 added (511 base + 26 ext) Split hasher chiplet into controller + permutation sub-modules. The single 113 of the 122 updated entries are category-3 (identical source, upstream gate recomputed) and fall under the narrative above; they are omitted from the table below. The 9 non-trivial entries are listed: Updated (non-trivial):
Removed:
Added:
46. refactor: restructure hasher controller constraints by operation lifetime8 updated | 3 removed | 1 added (509 base + 26 ext) Lifetime-based restructure of hasher controller constraints. Collapses Updated (non-trivial):
Removed:
Added:
|
Two leftovers from the constraint-simplification work that don't belong in the merge to next: - CONSTRAINT_CHANGES.md: developer log of constraint polynomial deltas across the refactor. Already archived as PR #2856 issue comment 4242623244 (the deduped version), which is the authoritative copy. Zero code references. - .constraint-tooling/recorder/degree_builder.rs: debug-only helper for tracking polynomial degrees. Its own docstring describes it as scratch ("not compiled into the miden-air crate ... revert the edits once you're done so the debug-only code doesn't ship"). Zero references in the repo; not in any mod.rs or Cargo.toml. The .gitignore entry for .constraint-tooling/ is kept so future local runs of the recorder/differ don't dirty the working tree. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Constraint simplification
Add a dedicated HASHER_PERM_LINK bus prefix (bus index 9, NUM_BUS_TYPES bumped to 10) and route both the AIR perm-link encoder and the aux-trace builder through `Challenges::encode` instead of the hand-rolled `alpha + sum(beta^i * h[i])` forms. This was the last bypass of the new per-bus domain-separation scheme: previously the perm-link messages shared the `v_wiring` column with ACE wiring and memory range checks but used bare `alpha`, so label collisions across contributors were theoretically possible. Also fixes the stale hasher bus layout docstrings in `aux_trace/bus/hasher.rs` that described the encoding as `alpha + <beta, ...>` (per Nashtare review). Regenerated with `regenerate-constraints --write`: RELATION_DIGEST, constraints_eval.masm, and the insta snapshot all shift. num_inputs (572) and num_eval_gates (5580) unchanged - only the additive base of two v_wiring perm-link sub-expressions moves from `alpha` to `bus_prefix[HASHER_PERM_LINK]`. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace the post-hoc `for row in 0..bitwise_start` loop deriving `s_ctrl = 1 - perm_seg` with a single `trace[0][..hasher_ctrl_len].fill(ONE)` memset, matching how s1..s4 are already populated. The padded controller length comes from a new `Hasher::region_lengths()` accessor, refactored out of `estimate_trace_len` so the layout math has a single source of truth - both callers go through the same `(ctrl_padded, perm_len)` computation. The hasher keeps writing `perm_seg` (trace[20]) row-by-row as part of its existing per-row trace construction; no extra pass. The old loop walked the entire hasher region doing read-modify-write per row, the new code is a write-only SIMD-friendly memset over just the controller prefix. Also silences a `clippy::needless_range_loop` warning the old form triggered. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Remove three unit tests introduced alongside `AceDag::compact()` in 8d259ed. The pass was wiped out in the e875d3d merge from origin/next (which adopted the new DagBuilder architecture) and never reinstated; the tests either asserted orphans are kept (contradicting their name) or referenced a no-longer-existing compaction step. Dropping them makes unit_tests.rs byte-identical to origin/next. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
4efb086 to
16246ba
Compare
huitseeker
left a comment
There was a problem hiding this comment.
Amazing, thanks so much!
Al-Kindi-0
left a comment
There was a problem hiding this comment.
Great work!
Mostly nits and a few questions for some followups.
There is also a number of stale comments related to the encoding of messages which need updating to the new prefix-based manner.
Fix review comments from Al-Kindi-0 and Nashtare on PR #2856: - Use OpFlags::u32_rc_op() in range bus instead of manual recomputation - Replace PermutationCols _ fields with private _unused array + accessor - Add ControllerCols::f_mu/f_mv and AceCols::f_read/f_eval flag methods - Add last-row boundary constraint enforcing HALT on final decoder row - Fix comment/doc nits: alignment, terminology, stale references - Update bus message format comments to use bus_prefix[CHIPLETS_BUS] - Use assert_eq_ext instead of assert_zero_ext(lhs - rhs) - Rename (re, im) to (a0, a1) for extension field elements Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Fix review comments from Al-Kindi-0 and Nashtare on PR #2856: - Use OpFlags::u32_rc_op() in range bus instead of manual recomputation - Replace PermutationCols _ fields with private _unused array + accessor - Add ControllerCols::f_mu/f_mv and AceCols::f_read/f_eval flag methods - Add last-row boundary constraint enforcing HALT on final decoder row - Fix comment/doc nits: alignment, terminology, stale references - Update bus message format comments to use bus_prefix[CHIPLETS_BUS] - Use assert_eq_ext instead of assert_zero_ext(lhs - rhs) - Rename (re, im) to (a0, a1) for extension field elements Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
a53eef4 to
d2326eb
Compare
Restore semantic detail from the original ace.rs column docs that was lost during the struct refactor: explain what the ACE chiplet does, its two-phase operation model (READ/EVAL), the role of multiplicity columns in the wiring bus, and the physical column aliasing between READ and EVAL overlays. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Al-Kindi-0
left a comment
There was a problem hiding this comment.
Looks great!
Left a couple of additional comments, otherwise all good from my end
- Fix stale `alpha` references in hash_kernel doc comments to `bus_prefix[bus]` - Rename misleading `alpha` variable to `prefix` in wiring range-check term - Unify `perm_seg` naming to `s_perm` everywhere (field, constants, methods, docs) - Add defensive boundary constraint: output at ctrl→perm boundary must be final (is_boundary=1), preventing non-final outputs from expecting continuations that never come since the next row belongs to the permutation segment Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…mplification # Conflicts: # air/src/constraints/stack/stack_arith/mod.rs
Adapt the Falcon-related u32add/u32sub constraint tests to the refactored API (MainCols, typed DecoderCols, OpFlags::new signature). Re-establish the stack_arith/ directory module to host the test file. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Fixes #2820 |
Pulls in the post-#2856 cleanup (clippy pass, PR review nits, HALT boundary constraint) plus u64/falcon fixes and CI hardening. Legacy bus/aux_trace files touched on origin/next stay deleted (superseded by the all-LogUp refactor on this branch). perm_seg was renamed to s_perm in MainCols + MainTrace; call sites in lookup/buses and trace tests updated. ACE circuit regenerated against the merged constraints (5512 eval gates, new RELATION_DIGEST propagated to config.rs, mod.masm, constraints_eval.masm, and the insta snapshot). All 3005 workspace tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Describe your changes
This PR aims to simplify our current constraints. To ease review, each commit can be examined separately to ensure transformations are semantics-preserving.
WIP
FG's intro
https://gisthost.github.io/?d05c574a2bdbc518ca1bc404f3d02409
Checklist before requesting a review
nextaccording to naming convention.