feat(math): add poset closure, dual, and induced subposet operations - #2039
feat(math): add poset closure, dual, and induced subposet operations#2039morluto wants to merge 4 commits into
Conversation
…re, boundary, Kolmogorov quotient, and continuity check Add the finite_topology_spaces domain implementing exact, bounded, deterministic finite topological space operations over immutable finite topological spaces represented by specialization preorders: - topology.finite.interior.compute: largest open set contained in a subset - topology.finite.closure.compute: smallest closed set containing a subset via specialization preorder up-set - topology.finite.boundary.compute: closure minus interior - topology.finite.kolmogorov_quotient.compute: T0 quotient identifying points with the same minimal open neighbourhood - topology.finite.continuity_check.compute: continuity check via specialization preorder monotonicity (x' <= x implies f(x') <= f(x)) On a finite set, every topology is Alexandrov, so the topology is equivalently represented by its specialization preorder. The FiniteTopologicalSpace value parses only well-formed spaces with reflexive preorders and in-range indices. Closes #1920
…ests - Import TOOLS and ADMISSIONS in catalog/builtins.py. - Update frozen admission baselines and add the finite_topology_spaces schema-snapshot fragment (5 operations). - Add 14 focused tests (interior, closure, boundary, Kolmogorov quotient, continuity check, validation).
Add four exact operations for canonical integer multiplicative normalization: - integer.perfect_power.profile.compute: maximal perfect-power profile with structural zero/unit/nonunit classification, handling negative integers by restricting to odd maximal exponents. - integer.k_free_decomposition.compute: canonical n = a^k * c where a >= 1, c has the sign of n, and no prime to the k-th power divides |c|. - integer.squarefree_decomposition.compute: direct k=2 specialization with square factor, signed squarefree part, and exponent parity rows. - quadratic_radical.positive_integer.normalize.compute: canonical sqrt(n) = s * sqrt(d) with ZERO/RATIONAL/IRRATIONAL classification. All operations use SymPy factorint as a private backend and return Jacobian-owned typed values with exact reconstruction validators. Closes #1893
…1746) Add four exact operations extending the finite-poset domain: - poset.lower_closure.compute: compute ↓S = {x : x ≤ s for some s in S} - poset.upper_closure.compute: compute ↑S = {x : s ≤ x for some s in S} - poset.dual.compute: return the order-reversed poset with transport map - poset.induced_subposet.compute: restrict to a subset with full order/cover structure All operations consume and return the authoritative FinitePoset value. Closes #1746
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| # Filter covers | ||
| covers = tuple( | ||
| OrderedPair(lower=p.lower, upper=p.upper) | ||
| for p in poset.cover_relations | ||
| if p.lower in subset_set and p.upper in subset_set | ||
| ) |
There was a problem hiding this comment.
🔴 Restricting a poset to certain subsets fails with an internal error instead of returning a result
The order links of the smaller poset are copied straight from the original (poset.cover_relations filter at src/jacobian/math/posets/_closure_operations.py:150-154) instead of being recomputed for the smaller set, so any request whose subset skips an intermediate element aborts with an internal error.
Impact: Users asking for the subposet on, e.g. the top and bottom of a chain, get a crash rather than the subposet.
Cover relations are not the transitive reduction of the restricted order
FinitePoset.require_complete_canonical_poset (src/jacobian/math/posets/_models.py:379-380) rejects any poset whose cover_relations is not exactly the transitive reduction of strict_order_pairs. For the chain a<b<c with subset {a, c}: the filtered strict pairs are {(a,c)} but the filtered covers are empty because both original covers (a,b) and (b,c) touch the removed element b. The reduction of {(a,c)} is {(a,c)} != {} so FinitePoset(...) at src/jacobian/math/posets/_closure_operations.py:188-198 raises ValueError, which escapes induced_subposet as a host exception. The existing tests only use subsets that happen to be cover-closed, so this is not caught. The covers must be recomputed as the transitive reduction of the restricted strict order (the helpers _strict_closure / _transitive_reduction in src/jacobian/math/posets/_models.py:75-103 already do this).
Prompt for agents
induced_subposet in src/jacobian/math/posets/_closure_operations.py builds the new poset's cover_relations by filtering the parent's cover_relations to pairs whose endpoints are both in the subset. That is not the transitive reduction of the restricted order whenever the subset omits an intermediate element (e.g. chain a<b<c restricted to {a,c}), and FinitePoset's validator (src/jacobian/math/posets/_models.py, require_complete_canonical_poset) rejects such a poset with ValueError, turning an accepted request into a host exception. Recompute the covers as the transitive reduction of the restricted strict order (the module-private helpers _strict_closure/_transitive_reduction in _models.py already implement this) and add tests for non-convex subsets such as {a,c} of a 3-chain.
Was this helpful? React with 👍 or 👎 to provide feedback.
| ranks = canonical_poset_ranks(elements, {(p.lower, p.upper) for p in covers}) | ||
| ranked = poset.graded and all( | ||
| e in {r.element for r in poset.ranks} if poset.ranks else True for e in elements | ||
| ) |
There was a problem hiding this comment.
🔴 Restricting a graded poset to a subset that is no longer graded fails with an internal error
The smaller poset inherits the "graded" flag from the original poset (ranked = poset.graded and all(...) at src/jacobian/math/posets/_closure_operations.py:174-176) instead of being tested on its own, so a restriction that is genuinely not graded aborts with an internal error.
Impact: Legitimate subposet requests fail with a crash instead of returning the restricted poset.
The `all(...)` guard is vacuously true and the rank recomputation may disagree
poset.ranks always covers every parent element, and elements is a subset of them, so the all(...) term is always True and ranked is exactly poset.graded. But ranks = canonical_poset_ranks(...) (line 173) is recomputed on the restricted cover relation and can return None for a graded parent. Example: the Boolean lattice on {1,2,3} (graded) restricted to {1, 12, 123, 23}; here the filtered covers do coincide with the reduction, but 123 has lower covers 12 (rank 1) and 23 (rank 0) in the subposet, so canonical_poset_ranks returns None. The construction then passes graded=True, ranks=None to FinitePoset (lines 188-198), and _validate_poset_rank_structure (src/jacobian/math/posets/_models.py:325-326) raises "graded metadata does not match the canonical poset", escaping as a host exception.
The grading flag should be derived from the recomputed ranks (ranked = ranks is not None).
| ranks = canonical_poset_ranks(elements, {(p.lower, p.upper) for p in covers}) | |
| ranked = poset.graded and all( | |
| e in {r.element for r in poset.ranks} if poset.ranks else True for e in elements | |
| ) | |
| ranks = canonical_poset_ranks(elements, {(p.lower, p.upper) for p in covers}) | |
| ranked = ranks is not None |
Was this helpful? React with 👍 or 👎 to provide feedback.
| _MAX_INTEGER_LENGTH = 256 | ||
| _MAX_EXPONENT = 1_000_000 | ||
|
|
||
|
|
||
| # --------------------------------------------------------------------------- | ||
| # Requests | ||
| # --------------------------------------------------------------------------- | ||
|
|
||
|
|
||
| class PerfectPowerProfileRequest(StrictModel): | ||
| """One canonical integer for the maximal perfect-power profile.""" | ||
|
|
||
| value: str = Field(pattern=r"^-?(?:0|[1-9][0-9]*)$", max_length=_MAX_INTEGER_LENGTH) | ||
|
|
||
|
|
||
| class KFreeDecompositionRequest(StrictModel): | ||
| """One integer and exponent k >= 2 for k-free decomposition.""" | ||
|
|
||
| value: str = Field(pattern=r"^-?(?:0|[1-9][0-9]*)$", max_length=_MAX_INTEGER_LENGTH) | ||
| k: int = Field(ge=2, le=10_000) | ||
|
|
||
|
|
||
| class SquarefreeDecompositionRequest(StrictModel): | ||
| """One integer for squarefree decomposition.""" | ||
|
|
||
| value: str = Field(pattern=r"^-?(?:0|[1-9][0-9]*)$", max_length=_MAX_INTEGER_LENGTH) | ||
|
|
||
|
|
||
| class QuadraticRadicalNormalizeRequest(StrictModel): | ||
| """One nonnegative integer for positive quadratic-radical normalization.""" | ||
|
|
||
| value: str = Field(pattern=r"^(?:0|[1-9][0-9]*)$", max_length=_MAX_INTEGER_LENGTH) |
There was a problem hiding this comment.
🔴 New integer normal-form operations accept 256-digit inputs and can run forever while factoring them
The new integer normal-form requests accept numbers up to 256 digits (max_length=_MAX_INTEGER_LENGTH at src/jacobian/math/number_theory/_normal_forms.py:32) and then fully factor them, so a single request can occupy a worker indefinitely.
Impact: One ordinary-looking request with a large hard-to-factor number can hang the server.
Unbounded factorization work versus the domain's existing factorization bound
All four new requests (PerfectPowerProfileRequest, KFreeDecompositionRequest, SquarefreeDecompositionRequest, QuadraticRadicalNormalizeRequest, lines 29-51) use the 256-digit bound, and every kernel calls _factorint -> sympy.factorint(abs(n)) (src/jacobian/math/number_theory/_normal_form_operations.py:30-34). The existing number-theory domain deliberately caps any input that must be factored at 12 digits via FactorizationInteger (src/jacobian/math/number_theory/_models.py:52-59, with the comment that small bounds "keep arithmetic functions that may factor their input ... safe for in-process SymPy execution"). AGENTS.md requires separately bounding accepted input and algorithmic work; a 256-digit RSA-style semiprime makes factorint effectively non-terminating.
Prompt for agents
The four new requests in src/jacobian/math/number_theory/_normal_forms.py accept canonical integers up to 256 digits, but every kernel in _normal_form_operations.py calls sympy.factorint on |n|, which is unbounded work for large inputs. The domain already defines FactorizationInteger in src/jacobian/math/number_theory/_models.py with a 12-digit cap precisely for operations that must factor their input. Reuse that bounded annotated type (or an explicitly justified, tested bound derived from the mathematics) for the new value fields so the advertised request domain matches what the algorithm can exhaust.
Was this helpful? React with 👍 or 👎 to provide feedback.
| power divides |c|. | ||
| """ | ||
| n = int(request.value) | ||
| k = request.k |
There was a problem hiding this comment.
🟡 New integer operations parse and format wire numbers with raw conversions instead of the mandated canonical helpers
Wire integer strings are converted with plain int(...)/str(...) (for example n = int(request.value) at src/jacobian/math/number_theory/_normal_form_operations.py:120) instead of the repository's canonical parse/format helpers, breaking the documented contract for canonical decimal wire values.
Impact: Large-value handling diverges from every other integer operation in the project.
Rule and affected sites
AGENTS.md ("Types and transport"): "Canonical decimal strings are wire values, not computation values. Use the canonical parse/format helpers—never direct int() or str()—and test above 4,300 digits whenever the contract permits it." jacobian.canonical provides parse_canonical_integer and format_canonical_integer (see src/jacobian/canonical.py:31,48), which bypass CPython's 4300-digit string/int conversion limit.
Affected new code: src/jacobian/math/number_theory/_normal_form_operations.py:59-249 (int(request.value) in all four kernels, str(p), str(base_val), str(a), str(c), str(s), str(d)), and the reconstruction validators in src/jacobian/math/number_theory/_normal_forms.py:92,138-139,181-183,215-217 which also call int(...) on wire fields.
Prompt for agents
AGENTS.md mandates that canonical decimal wire strings are parsed and formatted with jacobian.canonical's parse_canonical_integer/format_canonical_integer helpers rather than bare int()/str(). The new files src/jacobian/math/number_theory/_normal_form_operations.py and src/jacobian/math/number_theory/_normal_forms.py use raw int()/str() throughout (kernels and model validators). Replace those conversions with the canonical helpers so behaviour matches the rest of the number-theory domain.
Was this helpful? React with 👍 or 👎 to provide feedback.
| for i in subset: | ||
| if not 0 <= i < len(space.points): | ||
| raise ValueError("subset index out of range") |
There was a problem hiding this comment.
🔴 Finite-topology subset requests with an out-of-range point index crash instead of being rejected
A point index that does not exist in the space is rejected only deep inside the computation (raise ValueError at src/jacobian/math/finite_topology_spaces/operations.py:59-60) rather than by the request contract, so an accepted request turns into an internal error.
Impact: Callers get an opaque host failure instead of a clear validation message, and the same bad input is silently ignored by the neighbouring operations.
Request model does not constrain the subset; kernels disagree on handling
SubsetRequest.subset is an unconstrained tuple[int, ...] (src/jacobian/math/finite_topology_spaces/_models.py:18), with no bound against len(space.points) and no length cap. closure raises ValueError("subset index out of range"), which propagates out of compute_closure/compute_boundary as a host exception; interior (operations.py:41-50) never inspects the subset entries at all, so the same out-of-range or negative index is silently ignored and yields a result. AGENTS.md requires the request to encode the advertised mathematical domain and states that mathematical inapplicability belongs in the request validator or the result, not in a raised backend exception.
Prompt for agents
SubsetRequest in src/jacobian/math/finite_topology_spaces/_models.py declares subset as an unconstrained tuple[int, ...]. The closure kernel (operations.py) raises ValueError for out-of-range indices, which escapes as a host exception for a request the model accepted, while the interior kernel silently ignores such indices. Add a cross-field model validator on SubsetRequest that requires every subset index to be within range of space.points (and cap the subset length), then drop the in-kernel raise so validation lives in one place and the two kernels agree.
Was this helpful? React with 👍 or 👎 to provide feedback.
| @model_validator(mode="after") | ||
| def require_well_formed(self) -> Self: | ||
| if len(self.preorder) != len(self.points): | ||
| raise ValueError("preorder must have one row per point") | ||
| for row in self.preorder: | ||
| for idx in row: | ||
| if not 0 <= idx < len(self.points): | ||
| raise ValueError("preorder index out of range") | ||
| for i in range(len(self.points)): | ||
| if i not in self.preorder[i]: | ||
| raise ValueError("preorder must be reflexive") | ||
| return self |
There was a problem hiding this comment.
🟡 Finite topological spaces accept relations that are not real preorders and return wrong answers
The space contract only checks that each point is related to itself (require_well_formed at src/jacobian/math/finite_topology_spaces/values.py:30-41) and never checks transitivity or that point labels are distinct, so a relation that is not an order is accepted and every derived quantity is computed from it as if it were.
Impact: Interior, closure, boundary and quotient results can be silently wrong for such inputs.
Why transitivity matters here
The representation relies on rows of preorder being the minimal open neighbourhoods of an Alexandrov topology, i.e. the down-sets of a preorder. If the relation is not transitive, the family of down-sets is not a topology: interior (operations.py:41-50) can then return a set that is not open, and closure (operations.py:53-64) a set that is not closed, so the operations report wrong mathematical values without any signal. Duplicate labels in points are likewise unchecked despite the docstring stating "points are unique labels", which also makes kolmogorov_quotient produce ambiguous quotient point names (operations.py:102-104). AGENTS.md requires the request to encode the advertised mathematical domain—not only the JSON shape.
Prompt for agents
FiniteTopologicalSpace.require_well_formed in src/jacobian/math/finite_topology_spaces/values.py validates row count, index range and reflexivity, but not transitivity of the relation nor uniqueness of the point labels. The kernels in operations.py treat each preorder row as the minimal open neighbourhood of an Alexandrov topology, which is only valid for a transitive relation; otherwise interior/closure/boundary/kolmogorov_quotient return values that are not the topological quantities they claim. Extend the validator to require transitivity (row i must contain the rows of all its members) and unique point labels, and add boundary tests for both.
Was this helpful? React with 👍 or 👎 to provide feedback.
| from jacobian.math.finite_topology_spaces._admission import ( | ||
| ADMISSIONS as FINITE_TOPOLOGY_SPACES_ADMISSIONS, | ||
| ) | ||
| from jacobian.math.finite_topology_spaces._tools import ( | ||
| TOOLS as FINITE_TOPOLOGY_SPACES_TOOLS, | ||
| ) |
There was a problem hiding this comment.
🔍 PR bundles two unrelated feature sets beyond the stated poset work
The description covers only the four poset operations from issue #1746, but the diff also adds an entire new finite_topology_spaces domain (5 catalog operations, values/models/tools/admission) and four number-theory normal-form operations from issue #1893. CONTRIBUTING.md asks that each change stay focused on one outcome; these two extra feature sets carry their own contracts and admission rows and would benefit from separate review.
Was this helpful? React with 👍 or 👎 to provide feedback.
| OperationAdmission( | ||
| "topology.finite.continuity_check.compute", | ||
| AdmissionDecision.KEEP, | ||
| "exact continuity check via specialization preorder monotonicity", | ||
| ), |
There was a problem hiding this comment.
🔍 New continuity/specialization operations duplicate the existing finite_topology domain
The existing finite_topology domain already publishes topology.specialization_preorder.compute and topology.is_continuous.compute over an open-set representation (src/jacobian/math/finite_topology/_tools.py:31,71). The new domain adds topology.finite.continuity_check.compute with the same mathematical meaning over a preorder representation, and its admission rationale claims each row is "exact"/distinct. Two catalog entries answering the same question with different input encodings is a discoverability hazard for agents; consider consolidating or explicitly justifying the split in the admission rationale.
Was this helpful? React with 👍 or 👎 to provide feedback.
| __all__ = [ | ||
| "FiniteTopologicalMap", | ||
| "FiniteTopologicalSpace", | ||
| "boundary", | ||
| "closure", | ||
| "continuous_check", | ||
| "from_preorder", | ||
| "interior", | ||
| "kolmogorov_quotient", | ||
| "minimal_neighbourhoods", | ||
| "specialization_preorder", | ||
| ] |
There was a problem hiding this comment.
🔍 New native domain is not registered in the root jacobian.math namespace
finite_topology_spaces/__init__.py publishes a native API with __all__, but the domain is not added to jacobian/math/__init__.py's imports/__all__, and tests/math/public_api/test_namespace.py:18-44 pins the exact root domain list. The new tests/math/finite_topology_spaces/test_public_api.py only works because submodule import succeeds. Either register the domain in the root namespace (and the pinned list) or drop the native re-exports to avoid a half-public surface.
Was this helpful? React with 👍 or 👎 to provide feedback.
Deep review summaryVerdict: REQUEST CHANGES — induced subposets are not recomputed canonically and valid restrictions can raise internal errors. 1. Cover relations are filtered instead of recomputedThe induced strict order is correctly restricted, but covers are obtained by filtering the parent's cover relations. That is not the transitive reduction of the restricted order when an intermediate element is removed. For the chain Recompute covers as the transitive reduction of the restricted strict order and add a non-convex subset regression. 2. Gradedness is inherited instead of recomputed
Also canonicalize and validate subset uniqueness at the request boundary, and add a dual-of-dual identity/property test plus induced-subposet reconstruction tests rather than only cover-closed examples. The lower and upper closure formulas themselves are correct over the complete strict order. |
1 participant
Summary
Implements four operations from issue #1746: finite poset closures, dual, and induced subposet.
poset.lower_closure.computeCompute the lower closure ↓S = {x : x ≤ s for some s in S} for a subset S of a finite poset. Returns the closure as a sorted element set.
poset.upper_closure.computeCompute the upper closure ↑S = {x : s ≤ x for some s in S} for a subset S.
poset.dual.computeReturn the dual poset (order reversed) with the same element domain and an identity transport map. Minimal/maximal elements are swapped.
poset.induced_subposet.computeRestrict a poset to a subset of elements, computing the induced strict order pairs, cover relations, incomparable pairs, minimal/maximal elements, and rank structure.
Design choices
FinitePosetvalue from the existing poset domain.Test coverage
22 tests covering:
Closes #1746
Continue this on Linzumi