Genericize the algebra crate and resolution engine over the ground ring - #263
Closed
JoeyBF wants to merge 7 commits into
Closed
Genericize the algebra crate and resolution engine over the ground ring#263JoeyBF wants to merge 7 commits into
JoeyBF wants to merge 7 commits into
Conversation
The homological algebra was hardcoded to coefficients in the field F_p via the bit-packed `fp` crate. Introduce a base-ring abstraction so it can later run over any graded coefficient ring (e.g. F_2[tau] for C-motivic Ext) while keeping the classical F_p path bit-identical. Two coefficient-ring traits, cut so that defining an algebra needs strictly less than resolving over it: - `Ring` (algebra/base_ring.rs): the coefficient ring -- scalars, their arithmetic, `embed_field` for the F_p inclusion, and the representation of finite free modules over the ring (the `Vector`/`Slice`/`SliceMut` types and their ops). This is enough to define an algebra and act on its modules. - `GradedDvr` (linear_algebra/mod.rs): extends `Ring` with the linear algebra of *solving* -- images, kernels and quasi-inverses -- which is tractable precisely because the ring is a graded DVR (graded Nakayama). It is the base-ring-generic replacement for the `fp` row reduction the engine hardcodes. `Algebra` requires only `type BaseRing: Ring`, so an algebra can be defined over a ring whose solving linear algebra is not yet implemented; the stronger `GradedDvr` bound is imposed where resolution actually happens (`ModuleHomomorphism`, `FreeModuleHomomorphism`, and the types that store them). `Field` implements both traits by forwarding to `fp`, bottoming the recursion at itself. Module and homomorphism signatures are threaded over the base-ring scalar and the `BaseSlice` projections. `FreeModule`/`FreeModuleHomomorphism` store their outputs/kernels/images/quasi-inverses generically over the base ring; the remaining fp-matrix-backed module types stay pinned with `BaseRing = Field` and use the scalar coefficient directly, so relaxing a pin becomes a compile error to resolve rather than a silent bug. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JNkiiZghHggyMwDWfDn1y5
Lift the resolution step's inline linear algebra -- building the differential matrix, computing its kernel, and constructing the next stage -- onto the `GradedDvr` base-ring trait, and dispatch `step_resolution` through those methods instead of calling `fp` directly. `ChainComplex::Algebra` is bounded `BaseRing = Field`, so the classical engine monomorphizes to the existing `fp` code path (benchmarks are bit-identical); relaxing that single bound is the entry point for a non-field base ring. The base-ring scalar is threaded through the chain-complex, chain-homotopy, Yoneda and secondary layers, the resolution homomorphism, and the examples; the generic secondary-homotopy type carries the `GradedDvr` bound it needs to store a free-module homomorphism. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JNkiiZghHggyMwDWfDn1y5
A coefficient ring is itself an algebra over itself, so fold Ring into the Algebra hierarchy: Ring: Algebra. This makes Algebra::BaseRing: Algebra hold transitively (since BaseRing: Ring), so each graded piece of an algebra can be viewed as a module over its base ring — the basis for exposing motivic weight as the internal grading of the coefficient module. The coefficient capacity (scalar, vector, arithmetic) stays confined to the Ring sub-trait, keeping Algebra lean for the Steenrod, Milnor, Adem, and module algebras that are never coefficient rings. Pure supertrait addition: Field already implements Algebra, so this is a bound, not new boilerplate; all call sites and the monomorphized Field path are unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JNkiiZghHggyMwDWfDn1y5
The trait bounding a ring on which a free resolution can solve (compute images, kernels, quasi-inverses) is renamed GradedDvr -> Solvable, moving the emphasis from the sufficient condition (being a graded DVR) to the capability it grants. Pure identifier rename across the algebra and ext crates; no behavior change, all tests pass. The doc prose still explains that solvability requires a graded DVR. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JNkiiZghHggyMwDWfDn1y5
Give Algebra a concrete associated type GradedPiece: Module<Algebra = Self::BaseRing> and an accessor fn module_at(&self, t) -> Self::GradedPiece, so each graded piece of an algebra can be viewed as a module over its coefficient ring. Since BaseRing is itself an Algebra (the Ring: Algebra fold), "an R-module" is just Module<Algebra = R>, reusing the Module trait. The type is concrete (not -> impl Module) so the hand-rolled SteenrodAlgebra dispatch forwards it. For every classical algebra BaseRing = Field, and since every module over a field is free, the piece is simply a FreeModule<Field> with dimension(t) generators in degree 0 (weight-0 internal grading) -- no bespoke type needed, and it matches the plan's framing that free grades are free modules and non-free grades (motivic) are presentations. This is additive, engine-unused structural access; dimension(t) stays load-bearing. Behavior-preserving: all 69 resolution benchmarks are bit-identical, all algebra tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JNkiiZghHggyMwDWfDn1y5
- rustfmt (nightly): order associated type GradedPiece before methods in the Algebra impls, and restore alphabetical import ordering disturbed by the GradedDvr -> Solvable rename. - rustdoc (-D warnings): the linear_algebra module doc linked the private `algebra` module and used redundant explicit targets on [`Ring`] links; make the module reference plain and drop the redundant targets. just lint and just docs now pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JNkiiZghHggyMwDWfDn1y5
📝 WalkthroughWalkthroughThis PR introduces generic base-ring and scalar abstractions across algebras, modules, homomorphisms, chain complexes, and resolution construction. It adds field-backed linear-algebra solvers, updates algebra implementations and examples, and replaces literal coefficient handling with typed ring operations. ChangesGeneric algebra and linear algebra
Module and resolution migration
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![coderabbitai[bot]](https://cdn.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ext/crates/algebra/src/algebra/field.rs`:
- Around line 47-52: Extract the repeated graded-piece construction from
module_at in the relevant algebra implementations into a shared helper, such as
classical_graded_piece, accepting a Field base ring and generator count. Move
the FreeModule creation, add_generators, and compute_basis sequence into that
helper, then update module_at in field.rs, AdemAlgebra, and MilnorAlgebra to
call it with self.base_ring() and self.dimension(t).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6f10c759-831f-484e-9122-edf3d5c88e20
📒 Files selected for processing (32)
ext/crates/algebra/src/algebra/adem_algebra.rsext/crates/algebra/src/algebra/algebra_trait.rsext/crates/algebra/src/algebra/base_ring.rsext/crates/algebra/src/algebra/field.rsext/crates/algebra/src/algebra/milnor_algebra.rsext/crates/algebra/src/algebra/mod.rsext/crates/algebra/src/algebra/steenrod_algebra.rsext/crates/algebra/src/lib.rsext/crates/algebra/src/linear_algebra/mod.rsext/crates/algebra/src/module/finite_dimensional_module.rsext/crates/algebra/src/module/finitely_presented_module.rsext/crates/algebra/src/module/free_module.rsext/crates/algebra/src/module/homomorphism/free_module_homomorphism.rsext/crates/algebra/src/module/homomorphism/full_module_homomorphism.rsext/crates/algebra/src/module/homomorphism/generic_zero_homomorphism.rsext/crates/algebra/src/module/homomorphism/hom_pullback.rsext/crates/algebra/src/module/homomorphism/mod.rsext/crates/algebra/src/module/homomorphism/quotient_homomorphism.rsext/crates/algebra/src/module/module_trait.rsext/crates/algebra/src/module/quotient_module.rsext/crates/algebra/src/module/rpn.rsext/crates/algebra/src/module/suspension_module.rsext/crates/algebra/src/module/tensor_module.rsext/examples/sq0.rsext/examples/steenrod.rsext/src/chain_complex/chain_homotopy.rsext/src/chain_complex/finite_chain_complex.rsext/src/chain_complex/mod.rsext/src/resolution.rsext/src/resolution_homomorphism.rsext/src/secondary.rsext/src/yoneda.rs
JoeyBF
pushed a commit
that referenced
this pull request
Jul 10, 2026
Field, AdemAlgebra, and MilnorAlgebra all built their module_at graded piece with the same three lines (FreeModule::new + add_generators + compute_basis). Factor that into a pub(crate) classical_graded_piece helper in field.rs so the three classical impls share one source of truth. Behavior-identical (module_at is engine-unused); addresses the CodeRabbit review on #263. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JNkiiZghHggyMwDWfDn1y5
Field, AdemAlgebra, and MilnorAlgebra all built their module_at graded piece with the same three lines (FreeModule::new + add_generators + compute_basis). Factor that into a pub(crate) classical_graded_piece helper in field.rs so the three classical impls share one source of truth. Behavior-identical (module_at is engine-unused); addresses the CodeRabbit review on SpectralSequences#263. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JNkiiZghHggyMwDWfDn1y5
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ext/crates/algebra/src/algebra/milnor_algebra.rs (1)
1812-1824: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider testing at an odd prime as well.
The test covers p=2 only. Adding a p=3 case would exercise the generic (non-2-primary) path and provide broader coverage of
module_atcorrectness. This is non-blocking since the existing test suite and benchmarks already pass for odd primes.💚 Suggested additional test case
#[test] fn module_at_is_the_graded_piece() { - let algebra = MilnorAlgebra::new(ValidPrime::new(2), false); - algebra.compute_basis(20); - for t in 0..=20 { + for &p in &[2, 3] { + let algebra = MilnorAlgebra::new(ValidPrime::new(p), false); + let max_t = if p == 2 { 20 } else { 30 }; + algebra.compute_basis(max_t); + for t in 0..=max_t { let piece = algebra.module_at(t); // The graded piece is a free Field-module concentrated in degree 0, whose dimension // there equals the algebra's dimension in degree t. assert_eq!(piece.dimension(0), algebra.dimension(t), "p = {p}, t = {t}"); } + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ext/crates/algebra/src/algebra/milnor_algebra.rs` around lines 1812 - 1824, Add an odd-prime case, using ValidPrime::new(3), to the module_at_is_the_graded_piece test or a companion test, compute the basis, and assert each graded piece’s dimension matches algebra.dimension(t), covering the generic non-2-primary path.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@ext/crates/algebra/src/algebra/milnor_algebra.rs`:
- Around line 1812-1824: Add an odd-prime case, using ValidPrime::new(3), to the
module_at_is_the_graded_piece test or a companion test, compute the basis, and
assert each graded piece’s dimension matches algebra.dimension(t), covering the
generic non-2-primary path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ee68b4fd-5401-4c49-8c16-7af906034f17
📒 Files selected for processing (3)
ext/crates/algebra/src/algebra/adem_algebra.rsext/crates/algebra/src/algebra/field.rsext/crates/algebra/src/algebra/milnor_algebra.rs
JoeyBF
pushed a commit
that referenced
this pull request
Jul 10, 2026
Extend module_at_is_the_graded_piece to loop over p in {2, 3} so the
generic (non-2-primary) Milnor path is exercised as well. Test-only;
addresses a CodeRabbit coverage suggestion on #263.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JNkiiZghHggyMwDWfDn1y5
Collaborator
Author
|
We're exploring a way to handle motivic computations that doesn't depend on more general ground rings. We'll revisit if we need to. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The
algebracrate and theextresolution engine hardcoded the coefficientfield 𝔽ₚ throughout. This PR makes them generic over an arbitrary graded
ground ring, introducing a small trait tower so the same engine can resolve
over rings other than a field — while keeping the classical 𝔽ₚ path
behavior-preserving and bit-identical.
No new coefficient ring beyond 𝔽ₚ is added here; this is the abstraction layer
that a follow-up (C-motivic Ext over 𝔽₂[τ]) builds on.
Design
A coefficient ring is folded into the existing algebra hierarchy as a sub-trait,
and the "solving" linear algebra is split off into its own trait so it can be
required only where a resolution actually needs it:
Ring: Algebra(algebra/base_ring.rs) — a coefficient ring is analgebra, plus a scalar type (
Ring::Element), a representation of finite freemodules over it (
Ring::Vectorand slices), and by-value arithmetic. This isenough to define an algebra and act on its modules.
Solvable: Ring(linear_algebra/, new module) — the harder linearalgebra a free resolution needs: images, kernels, quasi-inverses, and the
resolution-step machinery. It is bounded only at
ModuleHomomorphism/FreeModuleHomomorphism/ChainComplex, so an algebra can be defined overa ring whose solving linear algebra isn't implemented yet.
Algebragainstype BaseRing: Ring+base_ring(), and amodule_at(t) -> GradedPieceaccessor exposing each graded piece as aModule<Algebra = BaseRing>.Coefficients and vectors are threaded generically via the aliases
Scalar<A>,VectorOf<A>,BaseSliceOf<A>,SubmoduleOf<A>,QuasiInverseOf<A>(allprojected through
A::BaseRing). The module actions, free modules, modulehomomorphisms, chain complexes, and resolution steps are rewritten against these
aliases instead of concrete
fptypes.Field(= 𝔽ₚ) is the reference implementer of bothRing(Element = u32,Vector = FpVector) andSolvable(every operation forwards tofp).Compatibility & validation
BaseRing = Fieldeverything monomorphizes to the same
fpcode as before. All 69 resolutionbenchmarks are bit-identical, and the full test suite passes.
cargo fmt --check,clippy(both--no-default-featuresand--all-targetsprofiles), and
cargo docunder-D warningsare all green.Algebra<BaseRing = Field>code is unaffected.Out of scope
ring, its solving linear algebra, and the C-motivic Steenrod algebra are a
follow-up that depends on this PR.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements