Fix Massey product panic on proper multiplication kernels (#116)#258
Conversation
`ExtAlgebra::massey_iter_c` and the single-shot `massey` built the
null-homotopy of `b ∘ c` per *generator* of the third-factor bidegree,
then computed the kernel of `·b` only afterwards. The null-homotopy of
`b ∘ gen_i` exists only when `b · gen_i = 0`, so whenever the kernel was
a proper subspace (some generator not individually killed by `b`) the
lift could not complete and `ChainHomotopy::extend` panicked ("Failed to
lift"). This surfaced for first factors of filtration `s >= 2`, e.g.
`<h3^2, d0, ->`.
Restructure both to realise the *actual* kernel class via
`ResolutionHomomorphism::from_class` and build a single valid
null-homotopy, mirroring the already-correct `massey_iter_a`:
- `massey_kernel` computes the valid third factors (kernel of `·b`) from
the product maps alone, no homotopy.
- `massey_bracket_of` realises one kernel class and reads its bracket.
- `massey` now verifies `b · c = 0` up front and never lifts an invalid
third factor.
This is no more expensive: `iter_c` now builds one homotopy per kernel
basis element (`dim(ker) <= num_gens`) instead of one per generator.
`massey_iter_a` is unchanged.
Adds regression tests: `<h1^2, h0, h0>` returns `None` without panicking,
and `massey_iter_c(h1^2, h0)` completes and agrees with `massey_iter_a`.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CAgVE4H22yxYTjSrH3r1iJ
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughRefactors Massey product computation in ChangesMassey product kernel refactor
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
`massey_representative`'s doc comment still linked `Self::massey_at`, which was removed in this change. CI builds docs with broken intra-doc links denied, so rustdoc failed. Point the doc at `massey_iter_a`, the remaining user of the answers-matrix path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CAgVE4H22yxYTjSrH3r1iJ
Take `b_hom` by value (`Arc<...>`) instead of `&Arc<...>`: the function hands it straight to `ChainHomotopy::new`, which consumes an owned `Arc`, so ownership is what it actually needs — callers now clone at the call site (a no-op move in the single-shot `massey`). Also fold the redundant `offset_a` argument, which was fully derived from `a`, into the function so `a` is self-sufficient. No behavior change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CAgVE4H22yxYTjSrH3r1iJ
The `offset_a` binding introduced in the previous commit was formatted by stable rustfmt; CI's lint job runs nightly rustfmt, which lays the method chain out differently. Reformat so `just lint` passes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CAgVE4H22yxYTjSrH3r1iJ
| if !resolution.has_computed_bidegree(prod_deg) { | ||
| return None; | ||
| } |
There was a problem hiding this comment.
We could also call compute_through_bidegree.
There was a problem hiding this comment.
I kept the None here for consistency with the rest of the Ext API: multiply_into/try_multiply and massey_bracket_of all return None on an uncomputed bidegree rather than resolving as a side effect, and the massey example resolves up front via compute_through_stem. Auto-computing only here would also be half a fix unless massey_bracket_of did the same for tot, and inside massey_iter_c it would silently extend the resolution mid-sweep. Happy to switch to on-demand compute_through_bidegree if you'd rather make that a deliberate API choice across the product/Massey helpers.
Generated by Claude Code
Per review: when the bracket bidegree `tot` is computed but has no generators, the Massey product lands in the zero group, so it is the (defined) zero element — not an undefined/uncomputed case. Return `Some` with a zero representative instead of `None`, skipping the null-homotopy. `None` now signals only an uncomputed `tot`. This only affects the single-shot `massey`; `massey_iter_c` already filters brackets containing zero, so its output is unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CAgVE4H22yxYTjSrH3r1iJ
* Fix Massey product panic on proper multiplication kernels (#116) `ExtAlgebra::massey_iter_c` and the single-shot `massey` built the null-homotopy of `b ∘ c` per *generator* of the third-factor bidegree, then computed the kernel of `·b` only afterwards. The null-homotopy of `b ∘ gen_i` exists only when `b · gen_i = 0`, so whenever the kernel was a proper subspace (some generator not individually killed by `b`) the lift could not complete and `ChainHomotopy::extend` panicked ("Failed to lift"). This surfaced for first factors of filtration `s >= 2`, e.g. `<h3^2, d0, ->`. Restructure both to realise the *actual* kernel class via `ResolutionHomomorphism::from_class` and build a single valid null-homotopy, mirroring the already-correct `massey_iter_a`: - `massey_kernel` computes the valid third factors (kernel of `·b`) from the product maps alone, no homotopy. - `massey_bracket_of` realises one kernel class and reads its bracket. - `massey` now verifies `b · c = 0` up front and never lifts an invalid third factor. This is no more expensive: `iter_c` now builds one homotopy per kernel basis element (`dim(ker) <= num_gens`) instead of one per generator. `massey_iter_a` is unchanged. Adds regression tests: `<h1^2, h0, h0>` returns `None` without panicking, and `massey_iter_c(h1^2, h0)` completes and agrees with `massey_iter_a`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CAgVE4H22yxYTjSrH3r1iJ * Fix broken intra-doc link to removed massey_at `massey_representative`'s doc comment still linked `Self::massey_at`, which was removed in this change. CI builds docs with broken intra-doc links denied, so rustdoc failed. Point the doc at `massey_iter_a`, the remaining user of the answers-matrix path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CAgVE4H22yxYTjSrH3r1iJ * Tidy massey_bracket_of signature Take `b_hom` by value (`Arc<...>`) instead of `&Arc<...>`: the function hands it straight to `ChainHomotopy::new`, which consumes an owned `Arc`, so ownership is what it actually needs — callers now clone at the call site (a no-op move in the single-shot `massey`). Also fold the redundant `offset_a` argument, which was fully derived from `a`, into the function so `a` is self-sufficient. No behavior change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CAgVE4H22yxYTjSrH3r1iJ * Apply nightly rustfmt to massey_bracket_of The `offset_a` binding introduced in the previous commit was formatted by stable rustfmt; CI's lint job runs nightly rustfmt, which lays the method chain out differently. Reformat so `just lint` passes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CAgVE4H22yxYTjSrH3r1iJ * massey: treat an empty target bidegree as a defined zero bracket Per review: when the bracket bidegree `tot` is computed but has no generators, the Massey product lands in the zero group, so it is the (defined) zero element — not an undefined/uncomputed case. Return `Some` with a zero representative instead of `None`, skipping the null-homotopy. `None` now signals only an uncomputed `tot`. This only affects the single-shot `massey`; `massey_iter_c` already filters brackets containing zero, so its output is unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CAgVE4H22yxYTjSrH3r1iJ --------- Co-authored-by: Claude <noreply@anthropic.com>
…quences#116) (SpectralSequences#258) * Fix Massey product panic on proper multiplication kernels (SpectralSequences#116) `ExtAlgebra::massey_iter_c` and the single-shot `massey` built the null-homotopy of `b ∘ c` per *generator* of the third-factor bidegree, then computed the kernel of `·b` only afterwards. The null-homotopy of `b ∘ gen_i` exists only when `b · gen_i = 0`, so whenever the kernel was a proper subspace (some generator not individually killed by `b`) the lift could not complete and `ChainHomotopy::extend` panicked ("Failed to lift"). This surfaced for first factors of filtration `s >= 2`, e.g. `<h3^2, d0, ->`. Restructure both to realise the *actual* kernel class via `ResolutionHomomorphism::from_class` and build a single valid null-homotopy, mirroring the already-correct `massey_iter_a`: - `massey_kernel` computes the valid third factors (kernel of `·b`) from the product maps alone, no homotopy. - `massey_bracket_of` realises one kernel class and reads its bracket. - `massey` now verifies `b · c = 0` up front and never lifts an invalid third factor. This is no more expensive: `iter_c` now builds one homotopy per kernel basis element (`dim(ker) <= num_gens`) instead of one per generator. `massey_iter_a` is unchanged. Adds regression tests: `<h1^2, h0, h0>` returns `None` without panicking, and `massey_iter_c(h1^2, h0)` completes and agrees with `massey_iter_a`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CAgVE4H22yxYTjSrH3r1iJ * Fix broken intra-doc link to removed massey_at `massey_representative`'s doc comment still linked `Self::massey_at`, which was removed in this change. CI builds docs with broken intra-doc links denied, so rustdoc failed. Point the doc at `massey_iter_a`, the remaining user of the answers-matrix path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CAgVE4H22yxYTjSrH3r1iJ * Tidy massey_bracket_of signature Take `b_hom` by value (`Arc<...>`) instead of `&Arc<...>`: the function hands it straight to `ChainHomotopy::new`, which consumes an owned `Arc`, so ownership is what it actually needs — callers now clone at the call site (a no-op move in the single-shot `massey`). Also fold the redundant `offset_a` argument, which was fully derived from `a`, into the function so `a` is self-sufficient. No behavior change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CAgVE4H22yxYTjSrH3r1iJ * Apply nightly rustfmt to massey_bracket_of The `offset_a` binding introduced in the previous commit was formatted by stable rustfmt; CI's lint job runs nightly rustfmt, which lays the method chain out differently. Reformat so `just lint` passes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CAgVE4H22yxYTjSrH3r1iJ * massey: treat an empty target bidegree as a defined zero bracket Per review: when the bracket bidegree `tot` is computed but has no generators, the Massey product lands in the zero group, so it is the (defined) zero element — not an undefined/uncomputed case. Return `Some` with a zero representative instead of `None`, skipping the null-homotopy. `None` now signals only an uncomputed `tot`. This only affects the single-shot `massey`; `massey_iter_c` already filters brackets containing zero, so its output is unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CAgVE4H22yxYTjSrH3r1iJ --------- Co-authored-by: Claude <noreply@anthropic.com>
Fixes #116.
Problem
ExtAlgebra::massey_iter_cand the single-shotExtAlgebra::masseypanic withFailed to lift(or, in release, theapply_quasi_inverseassertion) whenever the kernel of "multiply byb" at a third-factor bidegree is a proper subspace. The issue's reproduction still triggers it:(i.e.
a = h3²,b = d0), panicking atext/src/chain_complex/chain_homotopy.rs:247.Root cause
The triple Massey product
<a, b, c>is defined only whena·b = 0andb·c = 0.massey_atbuilt the null-homotopy of the compositef_b ∘ f_genᵢper generatorgenᵢof the third-factor bidegree, stored the per-generator bracket values in ananswersmatrix, and only afterwards computed the kernel of·b, combininganswerslinearly over the kernel rows.But the null-homotopy of
f_b ∘ f_genᵢ(multiplication byb·genᵢ) exists only whenb·genᵢ = 0in Ext. When some generator is not individually killed byb(the kernel is a proper subspace),ChainHomotopy::extendcannot lift and panics. The per-generator "answers matrix, then combine over the kernel" design is fundamentally invalid for proper kernels: the bracket is only defined on the kernel and can only be realised by building the homotopy for the actual kernel class. This only surfaced for a first factor of filtrations ≥ 2(the cycle assertion at the homotopy's top step is gated ontarget.s() > 1 == a.s() > 1), which is why the existing tests — all usingh0— never hit it.Fix
Restructure both entry points to realise the actual kernel class via
ResolutionHomomorphism::from_classand build a single valid null-homotopy, mirroring the already-correctmassey_iter_a:massey_kernel— the valid third factors (kernel of·b), computed from the product maps alone (no homotopy).massey_bracket_of— realises one kernel class and reads off its bracket.masseynow verifiesb·c = 0up front and never attempts to lift an invalid third factor.massey_at/MasseyComputeDatumare removed;massey_iter_aand the sharedmassey_result/massey_representative/massey_indeterminacyhelpers are unchanged.This is not more expensive:
massey_iter_cnow builds one null-homotopy per kernel basis element (dim(ker) ≤ num_gens) instead of one per generator.Testing
cargo clippy -p ext --libis clean; allextlibrary tests pass.(6,5)resolution already used by the existing tests):<h1², h0, h0>returnsNonewithout panicking (first factors = 2, exercising the single-shot path).massey_iter_c(h1², h0)completes and agrees with the independentmassey_iter_a(h0, h1²).🤖 Generated with Claude Code
Generated by Claude Code
Summary by CodeRabbit