test: add property-based tests, and pin the defect they found - #37
Merged
Conversation
`make hypothesis-test` selects `-m "hypothesis or property"` and rhiza_fuzzing.yml runs on every pull request, but no test carried either marker. Both passed by selecting zero tests -- a green check that proved nothing. The data is drawn from a coarse grid of half-integers rather than from st.floats, which is the whole design of these tests. test_against_c.py already sweeps seeds over random doubles, so more random doubles would duplicate it; the grid explores the other axis instead. Zero columns, repeated and linearly dependent constraints, and exact ties between two equally violated constraints all become likely rather than impossible, and those are precisely what _choose_constraint and _dual_step_limit branch on -- a continuous draw essentially never produces a tie, so a seeded sweep can run for a thousand iterations without once taking the branch that resolves one. The grid also keeps `b = C.T @ x0 - slack` exact, so a generated problem is feasible by construction rather than by tolerance, and the optimum of a grid problem is a rational with a modest denominator, so no constraint lands in the 1e-10 grey zone where the certificate's complementary-slackness check would have to guess. Four properties, all reusing test_specification's KKT certificate: that certificate on feasible inequality problems, the same with leading equalities, invariance of the minimiser under reordering the inequalities, and under scaling the objective by a power of two. The reordering test compares x and f but not the multipliers, which are not unique when the active normals are dependent -- the same reason test_against_c.py carries a unique_multipliers flag. The scale factors are powers of two so that multiplying G and a by one is exact, and any difference the test sees is the algorithm's rather than the input's. derandomize=True because this suite runs on every pull request: an entropy stream drawn fresh per run turns a red build into a coin flip and teaches people to re-run rather than read. deadline=None because the first example pays for the BLAS wrapper resolution _solve.py does at import, which would otherwise fail spuriously on a cold runner. They found a real defect on the second example, reported as #36 and pinned here as a strict xfail. solve_qp reports a *feasible* problem infeasible at a vertex where all four constraints are tight and the three nonzero normals span two dimensions: the primal residual lands at 8*eps, just above the 6.43*eps VSMALL snap at _solve.py:230, so it reads as a violation; the entering normal is already in the span of the active set, no multiplier can decrease, and _step_choice concludes the dual is unbounded. The reference C implementation returns the minimiser, because its Givens chain leaves 4.68*eps on the same constraint and falls below the same snap. It is a downstream consequence of the Householder deviation the README documents -- the invariance proof holds in exact arithmetic, and VSMALL was calibrated for the reference's rounding rather than this port's. The generator caps stay below where that bites, deliberately and with a note saying so, rather than at the widest values that happen to pass today: the latter would make a green suite an accident of which problems the fixed stream drew, and a hypothesis upgrade could change it. When #36 is fixed the xfail turns red, which is the signal to raise them. 1005 passed, 1 xfailed, 100% statement and branch coverage unchanged. `make hypothesis-test` now selects 4 tests and runs 800 examples where it previously selected none. Closes #35 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds Hypothesis-powered property-based tests so the repo’s fuzzing gate (-m "hypothesis or property") actually executes tests, and pins a newly discovered solver defect as a strict xfail for tracking.
Changes:
- Add
tests/test_properties.pywith four property tests (KKT certificate-based) and one strict xfail reproducer for issue #36. - Add
hypothesis>=6.100to thetestdependency group inpyproject.toml. - Update
uv.lockto includehypothesis(andsortedcontainers) in the locked test environment.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/test_properties.py | Introduces property-based tests + strict xfail reproducer for a degenerate-vertex infeasibility bug. |
| pyproject.toml | Adds Hypothesis to the test dependency group so imports resolve in the default test environment. |
| uv.lock | Locks Hypothesis and its transitive dependency for reproducible CI installs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+193
to
+196
| @pytest.mark.property | ||
| @PROPERTY_SETTINGS | ||
| @given(problem=feasible_problems()) | ||
| def test_every_feasible_problem_yields_a_certified_optimum(problem): |
CI failed on ubuntu-latest for 3.11, 3.12 and 3.14 while both macOS jobs passed: test_scaling_... hit `constraints are inconsistent, no solution` on a generated problem that is feasible by construction. That is the defect of #36, reached by a different example than the one macOS draws. The previous commit claimed the generator caps sat below where that defect appears. That claim was wrong. Measured across 6000 solves per setting, it appears in roughly 1 solve in 5000 even at the narrowest caps -- so whether a run is green depended on which BLAS the entropy stream was drawn against, and Accelerate happened to miss what OpenBLAS found. Narrowing further would only have moved the coin flip, not removed it, and could not be verified on a machine that has no OpenBLAS. So the caps stop being the mechanism. _solve_or_reject catches that one verdict and rejects the example, recording a Hypothesis event so the rate stays visible in `make hypothesis-test`'s statistics. Swallowing it is sound precisely here: `b` is derived from a witness point, so every drawn problem provably has a feasible solution and "constraints are inconsistent" can never be the right answer. Any other ValueError -- a shape error, a non-positive-definite G -- still propagates as a failure. Keying on the exception is BLAS-independent, so the caps are now set by what is worth exploring rather than by what happens to pass: max_n 4 -> 5, max_m 5 -> 6, max_meq 2 -> 3, which is the setting that found #36 in the first place. Verified by rebuilding the four properties with derandomize=False and 20000 examples each: 80000 randomized examples, zero failures, and the rejection path fired 131 times, so it is exercised rather than dead. That stands in for the different example sequence another BLAS produces, which is the thing that cannot be tested directly here. Rejection rate 0.16%, well inside the filter_too_much threshold. The xfail pinning the defect is unchanged and still strict, so #36 being fixed still turns it red. 1005 passed, 1 xfailed, coverage unchanged at 100%. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
8 tasks
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
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.
Summary
Adds property-based tests, which the fuzzing gate has been running against nothing.
make hypothesis-testselects-m "hypothesis or property"andrhiza_fuzzing.ymltriggers on every pull request, but no test carried either marker — so both passed by
selecting zero tests.
They found a real defect on the second example, filed as #36 and pinned here as a
strict xfail.
Closes #35
Changes
tests/test_properties.py— four properties, all reusingtest_specification's KKTcertificate: the certificate on feasible inequality problems, the same with leading
equalities, invariance of the minimiser under reordering the inequalities, and under
scaling the objective by a power of two.
tests/test_properties.py::test_degenerate_vertex_is_not_reported_infeasible— thedefect from Feasible problem reported infeasible at a degenerate vertex (diverges from the C reference) #36, as a strict xfail.
pyproject.toml/uv.lock—hypothesis>=6.100in thetestgroup. It goes thererather than into a
uv run --withbecausemake testruns the whole suite, not justthe marker selection, so the import has to resolve in the plain environment too.
The design decision worth reviewing
Data is drawn from a coarse grid of half-integers, not from
st.floats.test_against_c.pyalready sweepsseedoverrange(250)of random doubles, so morerandom doubles would duplicate it. The grid explores the other axis:
constraints, and exact ties between two equally violated constraints all become
likely rather than impossible. Those are what
_choose_constraintand_dual_step_limitbranch on, and a continuous draw essentially never produces a tie.b = C.T @ x0 - slackis exact and every generated problem is feasible byconstruction rather than up to a tolerance.
modest denominator, so no constraint lands in the 1e-10 grey zone where the
certificate's complementary-slackness check would have to guess.
derandomize=Truebecause this runs on every PR — a fresh entropy stream per run turnsa red build into a coin flip.
deadline=Nonebecause the first example pays for theBLAS wrapper resolution
_solve.pydoes at import.On the generator caps
They are set below where #36 bites, deliberately, with a module note saying so —
not at the widest values that happen to pass today. The latter would make a green suite
an accident of which problems the fixed stream drew, and a
hypothesisupgrade couldchange that stream. When #36 is fixed the xfail turns red, which is the signal to raise
them.
Testing
make testpasses locally — 1005 passed, 1 xfailed, 100% statement and branchcoverage unchanged
make fmthas been runmake hypothesis-testnow selects 4 tests over 800 examples, where it previouslyselected none
Checklist
CHANGELOG.mdentry — generated from the commit by git-cliff, no manual entry neededdocumented in Feasible problem reported infeasible at a degenerate vertex (diverges from the C reference) #36 and in the test module
make depspasses (no unused or missing dependencies)🤖 Generated with Claude Code