Add ExtAlgebra: a bigraded-algebra view over resolutions - #240
Conversation
|
Warning Review limit reached
More reviews will be available in 31 minutes and 26 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR adds a public ChangesExt algebra products
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/examples/product.rs`:
- Around line 30-32: The validation in the example’s zero-dimension branch
currently uses a panic in the product example, which aborts on expected invalid
user input. Update the logic around the dim check in the product example to
return a proper error from the surrounding function instead of panicking,
preserving the existing shift context in the error message and propagating it
through the caller’s Result flow.
In `@ext/src/ext_algebra.rs`:
- Around line 113-120: The public constructors in ExtAlgebra are not validating
basis coordinates or generator indices before they are used, which can later
panic when generator_product_map indexes class[g.idx()]. Add bounds/shape checks
in the public entry points such as element, unit_element, generator, and
generator_product_map so invalid coordinate lengths or out-of-range generator
indices are rejected early with a clear error instead of constructing invalid
classes or accessing non-existent generators.
- Around line 68-74: The constructor in new currently stores resolution, unit,
and is_unit without enforcing the expected invariants, which can leave later
product/dimension logic inconsistent. Add validation in new to ensure the
resolution and unit complexes are compatible when is_unit is true, and verify
both complexes share the same prime before any use by unit_element or product
arithmetic. Use the new constructor and self.prime() as the main entry points
for these checks so invalid Arc combinations are rejected early.
- Around line 183-190: Move the computedness guard in multiply_into ahead of the
dimension queries: check self.unit.has_computed_bidegree(b) and
self.resolution.has_computed_bidegree(target) before calling
number_of_gens_in_bidegree on unit and resolution. This keeps out-of-range
bidegrees safe and avoids the Resolution::module-backed lookup from panicking on
uncomputed degrees; then only allocate the result matrix after those checks
pass.
🪄 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: 3e7c6135-6621-420c-bf55-4a7e076f6dff
📒 Files selected for processing (3)
ext/examples/product.rsext/src/ext_algebra.rsext/src/lib.rs
Introduce `ext::ext_algebra::ExtAlgebra<CC>`, a thin ergonomics layer that presents Ext(M, k) as a bigraded module over the bigraded algebra Ext(k, k). It wraps a resolution (and the unit resolution) and exposes the bigraded basis plus products, so computing a product is a single `multiply`/`multiply_into` call instead of the manual ResolutionHomomorphism + extend + hom_k plumbing the examples re-derive. Products reuse the existing machinery: one ResolutionHomomorphism is built and cached per generator of Ext(M, k) (keyed by BidegreeGenerator), and a product by a general class is assembled at request time as the matching linear combination of generator maps. No linear-algebra core is reimplemented. `multiply_into` returns `Option<Matrix>` (rows = unit generators, columns = target generators), yielding `None` when a bidegree is out of the computed range rather than silently returning zeros; `try_multiply` is the corresponding safe variant and `multiply` panics out of range. Adds a streamlined `product` example and a unit test on S_2 (h_0^2 != 0, h_0 h_1 = h_1 h_0 = 0). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XYWcvWZPm3YJkVpCmeGYsP
6e67994 to
0f42179
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (2)
ext/src/ext_algebra.rs (2)
117-119: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winUse a strict upper bound for generator indices.
Line 118 allows
idx == ambient, but valid basis indices are0..ambient; this can pass validation and then panic or construct an invalid element downstream.Proposed fix
pub fn generator(&self, g: BidegreeGenerator) -> BidegreeElement { let ambient = self.dimension(g.degree()); - assert!(ambient >= g.idx()); - g.into_element(self.prime(), self.dimension(g.degree())) + assert!(g.idx() < ambient); + g.into_element(self.prime(), ambient) }🤖 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/src/ext_algebra.rs` around lines 117 - 119, The generator index check in the `ExtAlgebra::g` path is too permissive because `ambient >= g.idx()` allows `idx == ambient`; tighten the validation so generator indices are strictly less than the ambient dimension before calling `g.into_element`, using the existing `ambient` and `g.idx()` symbols to enforce the `0..ambient` range.
63-71: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winReject explicit unit resolutions over a different prime.
newaccepts arbitrary(resolution, unit)pairs, butprime()comes fromresolutionwhile unit dimensions/classes come fromunit; mismatched primes can produce invalid Ext elements/products.Proposed fix
pub fn new(resolution: Arc<CC>, unit: Arc<CC>) -> Self { + assert_eq!( + resolution.prime(), + unit.prime(), + "ExtAlgebra resolution and unit must have the same prime" + ); Self { is_unit: Arc::ptr_eq(&resolution, &unit), resolution,🤖 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/src/ext_algebra.rs` around lines 63 - 71, ExtAlgebra::new currently accepts mismatched resolution and unit classes, which can create invalid Ext elements when their primes differ. Update new to verify that resolution.prime() matches unit.prime() before constructing the struct, and reject or assert on invalid pairs; keep is_unit and the stored fields unchanged for valid inputs.
🤖 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.
Duplicate comments:
In `@ext/src/ext_algebra.rs`:
- Around line 117-119: The generator index check in the `ExtAlgebra::g` path is
too permissive because `ambient >= g.idx()` allows `idx == ambient`; tighten the
validation so generator indices are strictly less than the ambient dimension
before calling `g.into_element`, using the existing `ambient` and `g.idx()`
symbols to enforce the `0..ambient` range.
- Around line 63-71: ExtAlgebra::new currently accepts mismatched resolution and
unit classes, which can create invalid Ext elements when their primes differ.
Update new to verify that resolution.prime() matches unit.prime() before
constructing the struct, and reject or assert on invalid pairs; keep is_unit and
the stored fields unchanged for valid inputs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: aca146c6-7502-4191-8a71-74b7a645a7ad
📒 Files selected for processing (1)
ext/src/ext_algebra.rs
* Add ExtAlgebra: a bigraded-algebra view over resolutions Introduce `ext::ext_algebra::ExtAlgebra<CC>`, a thin ergonomics layer that presents Ext(M, k) as a bigraded module over the bigraded algebra Ext(k, k). It wraps a resolution (and the unit resolution) and exposes the bigraded basis plus products, so computing a product is a single `multiply`/`multiply_into` call instead of the manual ResolutionHomomorphism + extend + hom_k plumbing the examples re-derive. Products reuse the existing machinery: one ResolutionHomomorphism is built and cached per generator of Ext(M, k) (keyed by BidegreeGenerator), and a product by a general class is assembled at request time as the matching linear combination of generator maps. No linear-algebra core is reimplemented. `multiply_into` returns `Option<Matrix>` (rows = unit generators, columns = target generators), yielding `None` when a bidegree is out of the computed range rather than silently returning zeros; `try_multiply` is the corresponding safe variant and `multiply` panics out of range. Adds a streamlined `product` example and a unit test on S_2 (h_0^2 != 0, h_0 h_1 = h_1 h_0 = 0). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XYWcvWZPm3YJkVpCmeGYsP
3 participants
Introduce
ext::ext_algebra::ExtAlgebra<CC>, a thin ergonomics layer thatpresents Ext(M, k) as a bigraded module over the bigraded algebra Ext(k, k).
It wraps a resolution (and the unit resolution) and exposes the bigraded basis
plus products, so computing a product is a single
multiply/multiply_intocall instead of the manual ResolutionHomomorphism + extend + hom_k plumbing the
examples re-derive.
Products reuse the existing machinery: one ResolutionHomomorphism is built and
cached per generator of Ext(M, k) (keyed by BidegreeGenerator), and a product by
a general class is assembled at request time as the matching linear combination
of generator maps. No linear-algebra core is reimplemented.
This is the foundational slice; the secondary d2 differential and Massey
products are planned follow-ups. Adds a streamlined
productexample and a unittest on S_2 (h_0^2 != 0, h_0 h_1 = h_1 h_0 = 0); products cross-checked against
the independent
filtration_oneoutput.Co-Authored-By: Claude Opus 4.8 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01XYWcvWZPm3YJkVpCmeGYsP
Summary by CodeRabbit
try_multiply) and panicking (multiply) multiplication.