fix(experiments): reject an OMARS design that never varies a factor - #494
Open
kgdunn wants to merge 1 commit into
Open
fix(experiments): reject an OMARS design that never varies a factor#494kgdunn wants to merge 1 commit into
kgdunn wants to merge 1 commit into
Conversation
omars_properties checked that each factor reaches the middle level at least once, so that its pure quadratic is not the constant column a two-level factor gives. The mirror case was not checked: a factor left at the middle level in every run has the constant quadratic 0. Both are inestimable, and only the first was being caught. The consequence was worse than a permissive verifier. max_second_order_correlation skips constant columns, which is right on its own terms, but a pinned factor removes its own quadratic and all of its interaction columns from the comparison, so the score improves. generate_omars(selection_criterion= "min_second_order_correlation") minimises exactly that quantity, so it had a direct incentive to produce the degenerate designs the verifier was accepting, and did. Across a three-to-six factor sweep it returned one in roughly a third of cells, including a spurious perfect 0.000 at three factors in nine runs. The fix is to require an outer level as well as a middle one. With the verifier corrected the ILP rejects those candidates during the search and spends its budget on real ones instead: degenerate results fell from twelve to three, and two sizes that previously produced no usable design at all, three factors in nine runs and five factors in thirteen, now produce one. Five runs in three factors is the smallest case that shows the failure and is the new regression test: with the third factor pinned at the centre, is_omars returned True while the main-and-quadratic model matrix had rank 4 of 7. is_omars now returns False for matrices it previously accepted. Every such matrix has an inestimable quadratic, so no correct caller can be relying on the old answer, and it is corrected in place rather than deprecated. Verification: ruff check . and ruff format --check . clean mypy src/process_improve clean, 146 source files pytest -k omars: 291 passed, 2 skipped the 13-run definitive screening design still verifies Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W1daHPaXLs7PPRsq8CmMMm
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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
omars_propertieschecked that each factor reaches the middle level at least once, so that its pure quadratic is not the constant column a two-level factor gives. The mirror case was unchecked: a factor left at the middle level in every run has the constant quadratic0. Both are inestimable; only the first was caught, sois_omarswas returningTruefor designs whose main-and-quadratic model matrix is rank deficient.max_second_order_correlationskips constant columns, which is correct on its own terms, but a factor pinned at the centre removes its own quadratic and all of its interaction columns from the comparison, so the score improves.generate_omars(selection_criterion="min_second_order_correlation")minimises exactly that quantity, so it had a direct incentive to produce the degenerate designs the verifier was accepting, and did: across a three-to-six factor sweep it returned one in roughly a third of cells, including a spurious perfect0.000at three factors in nine runs.Behaviour change
is_omarsnow returnsFalsefor matrices it previously accepted. Every such matrix has an inestimable quadratic, so no correct caller can depend on the old answer. It is corrected in place rather than put through the deprecation schedule.Test plan
How this was verified:
test_factor_never_leaving_the_middle_is_not_omars, the mirror of the existingtest_full_factorial_is_not_omars. Five runs in three factors is the smallest case that shows it: with the third factor pinned at the centre,is_omarsreturnedTruewhile the main-and-quadratic model matrix had rank 4 of 7. The test asserts the rank deficiency alongside the verdict, and thatis_balancedandmain_effects_orthogonalare both stillTrue, so it is clear which check was the missing one.pytest -k omars: 291 passed, 2 skipped.ruff check .andruff format --check .clean;mypy src/process_improveclean over 146 source files.Checklist
pyproject.toml(PATCH for fixes/docs/config, MINOR for new features)ruff check .passesCHANGELOG.mdupdatedGenerated by Claude Code