Ext py#239
Draft
hoodmane wants to merge 169 commits into
Draft
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
hoodmane
force-pushed
the
ext_py
branch
8 times, most recently
from
4ff7f64 to
b6f09f7
Compare
Add a non-panicking accessor upstream returning Option<usize> (None =
uncomputed bidegree, so it is never confused with a computed Some(0)).
Use it in ext_py to replace the five hand-rolled num_gens clamping
helpers (Resolution/UnstableResolution::num_gens_at,
{,Unstable}ResolutionHomomorphism::target_num_gens, ext_algebra_num_gens),
mapping None to 0 to preserve the existing Python surface.
Add a non-panicking decompose variant upstream returning Result<_, DecomposeError>, where DecomposeError distinguishes OutOfRange (degree/idx do not name a basis element) from Indecomposable (the degree-0 unit or an algebra generator). Use it in ext_py to replace the hand-rolled degree==0/generators guard that reverse-engineered the upstream panic preconditions; behavior (error types/messages) unchanged.
Add a non-panicking coproduct variant upstream returning Result<_, CoproductError> (OutOfRange / OddPrimeUnsupported / IndivisibleDegree / NonzeroIndex). MilnorAlgebra and AdemAlgebra override it with their specific precondition guards; SteenrodAlgebra forwards via enum_dispatch. Use it in ext_py to delete the per-class coproduct guards (including the SteenrodAlgebra match over the Milnor/Adem union).
The struct (truncated: bool, q_part: u32, p_part: Vec<u32>) was trivially cloneable but didn't derive Clone, forcing ext_py to rebuild it field by field in four places. Derive Clone upstream and replace those rebuilds with .clone().
Add a type-erased field-element enum (over Fp and SmallFq FieldElements) to the fp crate, with the arithmetic operators implemented once: +,-,* return Option (None on mismatched field), try_div returns Result<_, DivError> distinguishing MismatchedField from DivisionByZero, plus inv/frobenius/neg/Display. Rewire ext_py's PyFieldElement to wrap it, deleting the local FieldElementKind enum and ~20 hand-written match arms. Behavior (results, exception types/messages, hash/eq) unchanged.
compute_kernel/compute_image/compute_quasi_inverse(s) panic with a slice OOB when called on a matrix whose pivots are uninitialized (not row-reduced). Add try_compute_* variants returning Option (and AugmentedMatrix<3>::try_compute_quasi_inverses returning Result<_, Self> so the matrix is recoverable on failure); leave the panicking methods and all existing callers untouched. Use them in ext_py to replace the ensure_pivots_initialized guard at its call sites; same ValueError and not-consumed-on-failure behavior.
QuasiInverse::new does no validation, so a Python-supplied image/preimage with more non-negative pivots than preimage rows (or an out-of-range pivot) made QuasiInverse::apply panic out of bounds. Add try_new returning Result<_, QuasiInverseError> (PivotOutOfRange / TooManyPivots); leave the unchecked new and its internal callers untouched. Use it in ext_py to replace the hand-rolled invariant re-derivation; same ValueError messages.
to_vec splits the buffer with itertools::chunks(stride), which panics when stride == 0 (zero columns). Short-circuit that case to return rows empty rows (the natural result; a 5x0 matrix is 5 empty rows), add a unit test, and drop the binding-side zero-column special-case.
Both new constructors assert on invalid input (panicking across the PyO3 boundary). Add additive try_new variants returning Result with error enums (SmallFqError: DegreeTooSmall/FieldTooLarge, using checked_pow to avoid overflow; AffineSubspaceError: PrimeMismatch/LengthMismatch); leave the asserting new and its callers untouched. Use them in ext_py to drop the hand-rolled pre-checks; same ValueError messages.
The borrowing StemIterator/iter_nonzero_stem can't live in a #[pyclass]. Add OwnedStemIterator<CC> + iter_stem_owned/iter_nonzero_stem_owned that hold an Arc<CC>, sharing a single stem_step cursor routine with the borrowing iterator so order/bounds can't drift. Use them in ext_py to replace the four hand-rolled stem walks (resolution + unstable resolution + chain complex iterators and the nonzero filter); same sequence/order/ filtering/termination.
Add try_module/try_differential (ChainComplex), try_chain_map (AugmentedChainComplex), try_get_map (MuResolutionHomomorphism), and try_homotopy (ChainHomotopy), each returning Option (None out of range) via the existing bound predicate so the panicking originals stay unchanged. Use them in ext_py to replace seven hand-rolled out-of-range guards; same exception types and messages.
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.
No description provided.