algebra: extract the Milnor product and derive generic from the prime - #292
algebra: extract the Milnor product and derive generic from the prime#292JoeyBF wants to merge 1 commit into
generic from the prime#292Conversation
|
Warning Review limit reachedNext included review available in 50 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Team Run ID: 📒 Files selected for processing (1)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Team Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe change removes stored generic-prime state and derives it from the prime. It extracts Milnor multiplication into the public ChangesMilnor product refactor
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This refactor centralizes Milnor multiplication and derives the prime-mode flag without changing the established algebra behavior or wrapper safeguards. No actionable merge-blocking risk remains beyond normal checks and review. Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
f961df1 to
3052fd3
Compare
|
@coderabbitai review |
|
3052fd3 to
7b9dc78
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
…r_product The motivic algebra engine landed on master with the Tau coefficient removed, so this tranche takes master's engine wholesale, drops the now dead tau.rs, and rewrites the A_C/tau view against the new API. A_C/tau is F_2[xi_i] tensor E(tau_i) — the odd-primary dual's shape with 2^i for p^i — so its product is the classical one at p = 2: the exterior commutation shifts by 2^k and the signs collapse over F_2. CTauAlgebra now multiplies through milnor_product (from the unmerged SpectralSequences#292, carried here) rather than filtering the engine's F_2[tau] product, so one algorithm stays under test instead of two. milnor_product takes its left factor first, commuting the right factor's exterior part past it; the engine's product_indexed orders its arguments the other way. The cross-check test pins this down: it asserts the two independent products — the classical machinery and the engine's Kong-Lin closed form — agree on the tau^0 part across every product up to total degree 5, and that some product really does drop a tau-divisible term. The engine also dropped its basis-element string API and its profiling counters, so the string round-trip moves here (inverting the Dual<Monomial> display) and the profile line loses its product counters. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013ePtYD7Bt4iPeCtmqtqvZE
Carve the motivic resolution layer out of the integration branch as the next tranche after the algebra engine (SpectralSequences#266). Adds CTauAlgebra, the A_C/tau view the ordinary resolution engine resolves, and MotivicResolution: resolve the trivial module over A_C/tau, then lift the differential to A_C by correcting along the weight grading. Includes the resolution cache and the resolve_motivic_ctau example. A_C/tau is F_2[xi_i] tensor E(tau_i) — the odd-primary dual's shape with 2^i for p^i — so its product is the classical one at p = 2: the exterior commutation shifts by 2^k and the signs collapse over F_2. It therefore multiplies through `milnor_product` (SpectralSequences#292) rather than reimplementing the walk, and a test cross-checks that against the engine's independent closed-form product, which is derived from Kong-Lin duality instead. Note that `milnor_product` takes its left factor first while the engine's `product_indexed` orders its arguments the other way; the two agree exactly under that transposition. Depends on SpectralSequences#292, which is not yet merged and so is included here. The Ext-side cohomology, the deformation spectral sequence, products and Massey products land in follow-ups. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013ePtYD7Bt4iPeCtmqtqvZE
`multiply_with_allocation` branched on `generic()`, but the two arms were the same computation: `multiply_qpart(m1, 0)` returns `[(1, m1)]`, so the classical arm is the generic one with an empty exterior part. Collapse them into `milnor_product`, a free function that reports each `(coefficient, basis element)` through a callback, leaving the index lookup and excess filter to the caller. `multiply_qpart` becomes free too; it only ever read the prime. `generic` was a `bool` field written once as `p != 2` and never mutated. Derive it instead. That also drops its `#[cfg(feature = "odd-primes")]` gate: without the feature `ValidPrime` is the zero-sized type 2, so the comparison folds to a constant on its own. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F25ZVbsP7ULg41iY3MP6FX
7b9dc78 to
fc807dc
Compare
1 participant
Pure refactor of the Milnor product path — no behaviour change, no new API surface beyond one free function.
Extract
milnor_productmultiply_with_allocationbranched ongeneric(), but the two arms were the same computation.multiply_qpart(m1, 0)returns[(1, m1)]—BitflagIterator::set_bit_iterator(0)yields nothing — so the classical arm is the generic arm with an empty exterior part. The two are collapsed into one free function:which reports each
(coefficient, basis element)through a callback, leaving the index lookup and excess filter to the caller.multiply_qpartbecomes free too; it only ever readself.p.MilnorAlgebra::multiply_with_allocationis now a thin wrapper.The motivation is downstream:$A_C/\tau \cong \mathbb{F}_2[\xi_i] \otimes E(\tau_i)$ has the odd-primary dual's shape with $2^i$ in place of $p^i$ , so its product is the same commutation rule plus one
PPartMultiplierwalk. Sharing this function rather than reimplementing it keeps one algorithm under test instead of two.Derive
genericfrom the primeThe field was written once by the constructor as
p != 2and never mutated — a cached derivation, not state.generic()now computes it.That also drops its$2$ , so
#[cfg(feature = "odd-primes")]gate. Without the featureValidPrimeis the zero-sized typeself.p != 2folds to a constantfalseon its own; the cfg was doing nothing the type system wasn't already doing.Removing the field also closes a latent trap. A hand-constructed$A_C/\tau$ (41 degrees checked, 0 differ), which makes it tempting as a shortcut — but $Q_k$ via $|Q_k| = 2p^k - 1$ . At $p = 2$ , $Q_k$ past $Q_0$ silently vanishes from the generating set. Correct at odd primes, unreachable through the constructor today, and now unreachable by construction.
p = 2, generic = truealgebra has exactly the graded dimensions ofgenerators()findsfactor_pk(p, degree + 1) == (k, 2), the odd-primaryfactor_pk(2, 2^{k+1}) = (k+1, 1), so everyChecks
cargo test -p algebra(63 pass),cargo fmt --check,cargo clippy --all-targets,cargo check --no-default-features, andjust docsunder CI'sRUSTFLAGS/RUSTDOCFLAGS: -D warnings— all clean.Relationship to the motivic work
Independent of #266 — the two touch disjoint regions of$A_C/\tau$ PR needs both.
milnor_algebra.rsand rebase past each other in either order. Only the follow-up🤖 Generated with Claude Code
https://claude.ai/code/session_01F25ZVbsP7ULg41iY3MP6FX
Summary by CodeRabbit
New Features
Improvements