feat(math): add integer multiplicative normal-form operations (#1893) - #2009
feat(math): add integer multiplicative normal-form operations (#1893)#2009morluto wants to merge 1 commit into
Conversation
Add five exact bounded operations for integer multiplicative normalization: - integer.perfect_power.profile.compute: maximal perfect-power profile with signed base/exponent and exact reconstruction, handling zero, units, positive, and negative integers correctly (negative integers use largest odd divisor of the exponent gcd). - integer.k_free_decomposition.compute: unique n = a^k * c decomposition with k-th-power-free cofactor, per-prime exponent rows, and exact reconstruction. - integer.squarefree_decomposition.compute: k=2 specialization with signed squarefree part and per-prime parity rows. - integer.squarefree_part.compute: compact projection returning the signed squarefree part and extracted square factor. - quadratic_radical.positive_integer.normalize.compute: canonical sqrt(n) = s*sqrt(d) normalization with squarefree d, classifying results as ZERO, RATIONAL_INTEGER, or IRRATIONAL_QUADRATIC. All operations use SymPy factorint for complete prime factorization and pure-Python arithmetic for the normal-form computation. Every result carries an exact reconstruction witness. Closes #1893.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
morluto
left a comment
There was a problem hiding this comment.
Review verdict: core formulas are correct, but the public contract is blocked and this overlaps #2038
I checked the arithmetic kernels across positive, negative, zero, and unit cases. The gcd-of-prime-exponents perfect-power rule (with the 2-adic part removed for negative inputs), e = qk+r k-free split, parity squarefree split, and positive quadratic-radical normalization are mathematically correct.
The blockers are in the value/catalog design:
1. Units are falsely classified as NONUNIT
Both k-free and squarefree operations send 1 and -1 through the nonzero branch, but the only nonzero result tag is "NONUNIT". The returned decompositions are numerically correct, yet the structural classification is false. Use NONZERO, or explicit unit variants.
2. The advertised closed variants are not closed
The validators only require fields when kind == "NONUNIT"; they do not forbid them otherwise and do not replay any equations. For example, a ZERO perfect-power result can carry an arbitrary base, exponent, factors, and reconstruction; a nonunit can carry a reconstruction unrelated to base**exponent; NormalizedQuadraticRadicalResult has no invariant validator at all. The in-kernel assert statements are not a wire invariant and disappear under optimized Python.
Bind at least:
base**exponent == source/reconstructionand maximality;base**k * cofactor == sourceplus all cofactor exponents< k;s**2*d == sourceplus squarefreeness/sign/canonicals >= 1;- radical classification, reconstruction, and squarefree radicand;
- absence of non-applicable fields on zero/unit variants.
3. integer.squarefree_part.compute is not the compact projection it claims to be
It has the same request, result schema, and implementation as integer.squarefree_decomposition.compute, including factor rows and reconstruction. This is a duplicate catalog alias, not a distinct atomic operation. Remove it or give it an actually smaller result value.
4. The operation is not computationally bounded
CanonicalInteger supplies no factorization-work budget here, while every operation calls complete factorint(abs(n)). An accepted adversarial semiprime can make the supposedly routine exact operation run without a defensible bound. Bound derived work/bit length to an empirically justified envelope or expose a bounded/incomplete factorization contract rather than calling arbitrary complete factorization bounded.
5. Overlap
#2038 implements the same four substantive operation IDs with stronger reconstruction validators and avoids the duplicate squarefree-part catalog entry. These branches cannot both merge. Prefer one domain owner and one schema; after its unrelated stacked changes are removed, #2038 is the better structural starting point, while this PR's reuse of the repository canonical-integer type is worth retaining.
So: no formula-level arithmetic error found, but I would not merge this head as a second implementation with false variants and unenforced normal-form invariants.
Deep review summaryVerdict: REQUEST CHANGES — the arithmetic formulas are correct, but the public variants/invariants are false or unenforced, the factorization work is unbounded, and this overlaps #2038. I checked positive, negative, zero, and unit cases. The gcd-of-prime-exponents perfect-power rule, odd-exponent restriction for negative inputs, The blockers are structural: 1. Units are falsely labelled
|
Summary
Add five exact bounded operations for integer multiplicative normalization, closing #1893.
Operations
Design
All operations use SymPy's for complete prime factorization, then pure-Python arithmetic for the normal-form computation (gcd, divmod, reconstruction). Every result carries an exact reconstruction witness (b^e = n, a^k * c = n, s^2 * d = n, etc.).
Key semantics
Tests
34 known-answer, boundary, and adversarial tests covering:
Continue this on Linzumi