feat(math): add integral binary quadratic forms domain - #2113
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6d305f6a5e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 37ad404a5e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fb2cc869cf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if ( | ||
| self.a == self.reduced_a | ||
| and self.b == self.reduced_b | ||
| and self.c == self.reduced_c | ||
| ): | ||
| return self |
There was a problem hiding this comment.
When the input is already reduced, this early return skips both the determinant and transformation checks. For example, a result with identical original/reduced coefficients but matrix=((2, 0), (0, 2)) is accepted even though the advertised SL₂(Z) witness has determinant 4, so corrupted producer output can pass result validation; validate the matrix on this path as well.
AGENTS.md reference: AGENTS.md:L103-L114
Useful? React with 👍 / 👎.
| @model_validator(mode="after") | ||
| def bind_equivalence(self) -> Self: | ||
| if self.status == "PROPERLY_EQUIVALENT" and self.matrix is not None: | ||
| p, q = self.matrix[0] |
There was a problem hiding this comment.
When a result carries NOT_PROPERLY_EQUIVALENT, this condition performs no mathematical validation at all; even identical forms such as (1,1,1) and (1,1,1) are accepted with that false status. A producer regression would therefore escape result construction and expose an incorrect exact decision, so recompute or otherwise bind both status branches to the supplied forms.
AGENTS.md reference: AGENTS.md:L74-L77
Useful? React with 👍 / 👎.
| if self.class_number != len(self.classes): | ||
| raise ValueError("class_number must equal the number of classes") |
There was a problem hiding this comment.
This checks only that class_number matches the supplied list length, not that the list is the complete class set promised by the operation. For example, ReducedClassesResult(discriminant=-23, classes=(), class_number=0) validates even though the exact class number is 3, allowing truncated or empty producer output to revalidate as an exact result; bind the tuple against the complete enumeration.
AGENTS.md reference: AGENTS.md:L103-L114
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4a03051bbf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| a: int | ||
| b: int | ||
| c: int |
There was a problem hiding this comment.
The form is represented here as three raw coefficients, again as form1/form2 tuples for equivalence, and as separate original/reduced fields in results. Consequently, even the serialized output of the check operation cannot be passed unchanged to evaluate or reduce because it contains extra status/result fields, forcing callers to reconstruct (a,b,c) manually. Introduce one domain-owned binary-quadratic-form value and use it consistently in producer results and consumer requests.
AGENTS.md reference: AGENTS.md:L95-L97
Useful? React with 👍 / 👎.
| a: int | ||
| b: int | ||
| c: int |
There was a problem hiding this comment.
Unlike x and y and the other coefficient-bearing requests, these fields accept integers of arbitrary magnitude. A native or successfully decoded request containing a coefficient with millions of digits therefore reaches _evaluate, which performs multiplication and constructs an equally unbounded exact result on the synchronous execution path. Apply an explicit coefficient budget in this request model so accepted inputs, intermediate work, and outputs remain bounded.
AGENTS.md reference: AGENTS.md:L128-L135
Useful? React with 👍 / 👎.
| if self.status == "PRIMITIVE_POSITIVE_DEFINITE": | ||
| if self.a is None or self.b is None or self.c is None: | ||
| raise ValueError("accepted form must carry coefficients") | ||
| if self.discriminant is None: | ||
| raise ValueError("accepted form must carry discriminant") | ||
| if self.discriminant != self.b**2 - 4 * self.a * self.c: | ||
| raise ValueError("discriminant must be b^2 - 4ac") |
There was a problem hiding this comment.
When status is PRIMITIVE_POSITIVE_DEFINITE, this validator checks only that the discriminant matches the coefficients; it never verifies a > 0, a negative discriminant, or primitivity. For example, status="PRIMITIVE_POSITIVE_DEFINITE", a=-1, b=0, c=1, discriminant=4 validates despite being indefinite, allowing an incorrect authoritative classification to revalidate as an exact result. Recheck the defining domain predicates in this branch.
AGENTS.md reference: AGENTS.md:L156-L157
Useful? React with 👍 / 👎.
…aluate, reduce, equivalence, and class enumeration Implement the primitive positive-definite integral binary quadratic form domain (issue #1830) with five exact operations: - number_theory.binary_quadratic_form.check: validate coefficients as a primitive positive-definite form with negative discriminant D ≡ 0 or 1 (mod 4), returning the discriminant and symmetric Gram matrix - number_theory.binary_quadratic_form.evaluate: exact evaluation Q(x,y) = a*x^2 + b*x*y + c*y^2 with primitive-pair status - number_theory.binary_quadratic_form.reduce: Gauss reduction to canonical reduced form with SL_2(Z) witness matrix and step ledger - number_theory.binary_quadratic_form.proper_equivalence.decide: decide proper equivalence by comparing canonical reduced representatives, returning the exact SL_2(Z) transformation witness - number_theory.binary_quadratic_form.reduced_classes.compute: enumerate all reduced primitive positive-definite classes of a given discriminant, returning the complete class set and class number h(D) Each result model re-runs the native kernel to verify exactness (fail-closed binding). All 22 known-answer, boundary, and adversarial tests pass.
4a03051 to
dc80790
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dc80790a78
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| @model_validator(mode="after") | ||
| def bind_result(self) -> Self: | ||
| if self.status == "PRIMITIVE_POSITIVE_DEFINITE": |
There was a problem hiding this comment.
When the status is NOT_IN_INITIAL_DOMAIN, the result omits the inspected coefficients and this validator performs no consistency check, so a result claiming that the valid form [1,1,1] is outside the domain can revalidate unchanged. Retain the source coefficients on this branch and verify the negative classification so the authoritative decision cannot be detached from its input.
AGENTS.md reference: AGENTS.md:L156-L157
Useful? React with 👍 / 👎.
| reduced_b: int | ||
| reduced_c: int | ||
| matrix: tuple[tuple[int, int], tuple[int, int]] | ||
| steps: tuple[tuple[int, int, int, int, int, int, int, int, int, int], ...] |
There was a problem hiding this comment.
The advertised reduction certificate includes this step ledger, but result validation never checks that each tuple transforms its recorded source to its destination, that consecutive steps chain together, or that their product equals matrix. Consequently, replacing a producer's ledger with arbitrary ten-integer tuples still yields a valid exact result; replay these bounded steps and their composition during validation.
AGENTS.md reference: AGENTS.md:L156-L157
Useful? React with 👍 / 👎.
1 participant
Summary
Implement the primitive positive-definite integral binary quadratic form domain (issue #1830) with five exact operations.
Operations
Design
Testing
Issue
Closes #1838 — integral binary quadratic forms: reduction, proper-class, composition, and representation operations.
Continue this on Linzumi