DSD: fix the conference-matrix construction, and add two-level categorical factors#475
Open
kgdunn wants to merge 17 commits into
Open
DSD: fix the conference-matrix construction, and add two-level categorical factors#475kgdunn wants to merge 17 commits into
kgdunn wants to merge 17 commits into
Conversation
… no silent fallback Placeholder commit; work follows.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…real DSD The conference-matrix constructor implemented Paley's construction only for a prime q = m - 1. At every other order it fell back to a cyclic approximation that is not a conference matrix, so the definitive screening designs for 9, 10, 15, 16, 21, 22, 25, 26, 27 and 28 factors had main effects correlated with each other at up to r = 0.9. generate_design returned those with a log warning only, and design_type="omars" shares the construction: it already reported omars_verified: False for the same counts. Paley's construction now works over any odd prime-power field, which covers 9, 10, 25, 26, 27 and 28 factors at minimal run size, and a verified order-16 table covers 15 and 16. The fallback is removed: when the minimal order has no conference matrix the constructor steps up to the next order it can build and records that in the metadata, and when nothing can be built it raises. Order 22 is the first order with no conference matrix at all, so 21 and 22 factors now cost 49 runs rather than 45. Every constructed matrix is checked against C.T @ C == (m - 1) * I before use. The upstream definitive_screening_design package shows why: its order-10 table has one mistyped entry, which leaves its 9- and 10-factor designs slightly non-orthogonal with nothing to catch it. Two adjacent errors found while auditing the same path: the planner estimated 2k + 1 runs for every DSD, undercounting every odd factor count, and it advertised a nonexistent fake_factor parameter on the strength of a comment that had the parity argument backwards. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TeEyMBnkeSKZ8QbSyejvNu
The coverage note described the cyclic fallback as a known limitation, the OMARS docstring pointed at it as the reason for its verification step, and the knowledge base advertised 2k + 1 runs for every factor count. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TeEyMBnkeSKZ8QbSyejvNu
Tests for the two reachable guards (Paley on a non-prime-power, and a request past the largest order the search considers). The third, a missing irreducible polynomial, cannot happen over a finite field and is marked as such. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TeEyMBnkeSKZ8QbSyejvNu
The repo's .coveragerc replaces coverage's default exclude_lines, so a bare "pragma: no cover" is not honoured here. Reach the guard with a stubbed irreducibility test instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TeEyMBnkeSKZ8QbSyejvNu
Two errors in the previous comment. The 2015 MATLAB port is Copyright (c) 2015 Jacob Albrecht, not Bristol-Myers Squibb; BMS is his affiliation and the organisation named in the BSD-3 no-endorsement clause. The Python package the table was read from is BSD 3-Clause, Copyright (c) 2022 Daniele Ongari, not MIT. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TeEyMBnkeSKZ8QbSyejvNu
The provenance, licensing, and the reason the table is verified rather than trusted now live in an attribute docstring on _CONFERENCE_MATRIX_16 itself, with an Attribution section and a reference to the originating paper. A comment block above an assignment is easy to detach from what it describes; a docstring travels with the object. _conference_matrix points at it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TeEyMBnkeSKZ8QbSyejvNu
test_registered_in_registry asserted that "test_dummy_add" and "test_dummy_mul" were present in the global registry, but those names are only registered when the sibling tests that apply the decorator actually run. Under pytest-xdist a sibling can be dispatched to a different worker process, whose registry this test never sees, so the assertion fails depending only on how the run happened to be split. The test now decorates its own function and asserts that name is registered, which is what the test is really about. Pre-existing on main and reproducible there; this branch surfaced it by adding tests, which changed the work split. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TeEyMBnkeSKZ8QbSyejvNu
Implements both column-augmentation procedures of Jones and Nachtsheim (2013). generate_design(design_type="dsd") previously raised as soon as a categorical factor was present, because the coded matrix was built as if every factor were continuous and the +/-1/0 values then failed the level check. A categorical factor takes a conference-matrix column like any other, so it costs the same runs as a continuous factor and contributes no quadratic term. The two zeros that column would carry have no meaning for a two-level factor, so each is resolved to a level, and extra centre runs are added because a centre run also needs a categorical setting. The methods differ in exactly that: DSD-augment sets z_j1 = b_j and z_j2 = -b_j, keeping the column balanced and every main effect unbiased by second-order effects; ORTH-augment sets both to +1, giving an orthogonal main-effects plan for up to four categorical factors at the price of aliasing main effects with categorical interactions. DSD-augment is the default. Its sign vectors are chosen by the determinant search the paper prescribes, not a fixed pattern. Verified against every oracle the paper provides: all 36 run sizes in Table 4, the constant-term alias row of Table 3 entry by entry, the information matrix of Equation (2), and Table 4's largest-alias column, which is 2/n throughout. The c = 2 panels of Figure 1 are not used as oracles. Both disagree with the rest of the paper: the DSD-augment panel gives the two categorical columns an inner product of 6, against the 2 of Equation (2) and the 0.1429 of Table 3, and is not the determinant-maximising choice its own Step 3 requires; the ORTH-augment panel prints a minus in run 10 where Step 3 requires a plus, leaving the column unbalanced and the main-effects plan non-orthogonal. Both findings are recorded as tests rather than prose. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TeEyMBnkeSKZ8QbSyejvNu
Codecov flagged eleven uncovered lines in the categorical work. Covering them turned up two things worth having. The large-c heuristic in the DSD-augment sign search had no test, so nothing checked that a design built without the exhaustive search is still definitive. It is, and now that is asserted. The index-coded branch of the categorical label mapping was reachable after all: dispatch_taguchi codes a categorical factor as level indices 0..n-1, and generate_design(design_type="taguchi") with a categorical factor raised on main because those indices reached the Column constructor unmapped. The mapping added for the DSD fixes it, and a regression test pins it. _is_prime became unreachable when _prime_power_factorization replaced it (the smallest divisor above one is always prime, so no separate test is needed). Ruff does not flag unused module-level functions, so it sat there looking load-bearing. Removed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TeEyMBnkeSKZ8QbSyejvNu
…m unverified design_type="omars" shares dispatch_dsd, so adding categorical support to the DSD silently extended it to OMARS as well. The result was a design carrying omars_verified: False, which is the exact silent-degradation failure the rest of this branch removes. On main the same call raised, further downstream, so this was a regression introduced here. OMARS is defined for quantitative factors: the family's properties are stated in terms of quadratic and interaction columns, and is_omars checks them that way. A two-level categorical factor has no quadratic, so a categorical DSD is a different object and belongs to design_type="dsd". The error message says so. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TeEyMBnkeSKZ8QbSyejvNu
PR #476 landed the score_contributions fix on main and took 1.61.0, which this branch had also claimed. Renumbered to 1.62.0 and collapsed this branch's two in-PR sections (1.60.1 and 1.61.0) into one, since the PR ships as a single version bump and neither intermediate version was ever tagged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TeEyMBnkeSKZ8QbSyejvNu
…elpers estimate_screening_runs had no way to express categorical factors, so its estimate for a definitive screening design under-counted by one to three runs whenever any were present. It now takes n_categorical and categorical_method and asks the constructor. The recommendation engine passes the number of two-level categorical factors specifically, since those are the only ones a DSD can carry; DOEProblemSpec.n_two_level_categorical exposes that count, distinct from n_categorical. The definitive-screening branch moved into its own helper rather than carrying a complexity suppression. dsd_run_count, dsd_conference_order and dsd_centre_runs are now exported from process_improve.experiments. Sizing a study before running it is a normal thing to want, and it should not require importing from a module named for response surfaces. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TeEyMBnkeSKZ8QbSyejvNu
A definitive screening design needs three factors and dsd_run_count raises below that, so the planner keeps the nominal 2k + 1 rather than letting the exception reach budget allocation for a design it would never recommend. That branch had no test; the accompanying assertion that the constructor itself still refuses is the half that matters. budget.py is now fully covered, statements and branches. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TeEyMBnkeSKZ8QbSyejvNu
A 502 from openmv.net failed the 3.11 job on this branch. The test is meant to skip when the host cannot be reached, and its docstring says so, but the guard had never worked: _read_remote_csv deliberately converts OSError and ValueError into a RuntimeError carrying an actionable message, while _load_or_skip caught only URLError, HTTPError and OSError. RuntimeError is not an OSError, so nothing the loader raises was ever caught, and any remote hiccup failed the build. Catching RuntimeError restores the documented behaviour. Nothing is weakened: when the host is up the test fetches and asserts in full, as it does locally. Two tests pin the helper itself, since the mismatch is invisible until an outage happens, which is the worst time to discover it. Neither file is touched by the rest of this branch; the failure was pre-existing and would have hit any PR unlucky enough to build during an outage. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TeEyMBnkeSKZ8QbSyejvNu
The sample datasets are fetched live from openmv.net, so a momentary 502 from that host failed an entire CI job on this branch. _read_remote_csv now makes three attempts with 1s and 2s of backoff, which rides out a blip at a cost of at most three seconds on a genuine outage. Only failures that could change on a second attempt are retried: connection errors, and 408, 425, 429 and the 5xx statuses. A 404 or a 403 will say the same thing next time, and a ValueError means the fetch succeeded and the content was wrong, so both are raised immediately. The error raised after the budget is exhausted is unchanged, so the callers that skip on it still skip. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TeEyMBnkeSKZ8QbSyejvNu
Codecov flagged a partial branch: the for loop's normal exit, which cannot happen because the body either returns or breaks on the last attempt. Rather than leave an uncoverable branch, the loop is now a while-True whose only exits are the return and the raise, so there is no fallthrough and no dead tail to carry. The one case the old shape did reach on that path was attempts < 1, which fell through with last_exc still None and reported "Could not download ...: None" - a failure that never happened. That is now rejected up front as the caller error it is. datasets.py is at 100%, statements and branches. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TeEyMBnkeSKZ8QbSyejvNu
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.
Two pieces of work on definitive screening designs. They touch the same function, which is why they share a PR.
Part 1: the construction was wrong at ten factor counts
dispatch_dsdreturned a design that is not a DSD for 9, 10, 15, 16, 21, 22, 25, 26, 27 and 28 factors._conference_matriximplemented Paley's construction only for a primeq = m - 1; at every other order it fell back to_cyclic_conference_matrix, which is not a conference matrix. Main effects came out correlated with each other at up to r = 0.9, returned with only a log warning.design_type="omars"shares the construction and already reportedomars_verified: Falseat the same counts.C.T @ C == (m - 1) * Ibefore use.Measured max off-diagonal of the Gram matrix (0 required):
Order 22 is the first order with no conference matrix at all: one of order
m ≡ 2 (mod 4)exists only whenm - 1is a sum of two squares, and 21 is not. So 21 and 22 factors now cost 49 runs. Every other count is unchanged. Jones & Nachtsheim (2013), p. 122, prescribe exactly this step-up.Part 2: two-level categorical factors
generate_design(..., design_type="dsd")raised as soon as a categorical factor was present. Both column-augmentation procedures of Jones & Nachtsheim (2013) are now implemented, selected with a newcategorical_methodargument."dsd"(DSD-augment, default)"orth"(ORTH-augment)z_{j,1}=b_j, z_{j,2}=−b_jz_{j,1}=z_{j,2}=1A categorical factor occupies a conference-matrix column like any other, so it costs the same runs as a continuous factor and contributes no quadratic term. Factors may be given in any order. Three-or-more-level categorical factors raise, pointing at
d_optimal.Verified against every oracle the paper provides
2/nacross all 36 rows. This is the check sensitive to the sign search that run sizes cannot detect.Two errata in the paper
The paper over-determines its own designs, which lets Figure 1 be cross-checked against Table 3, Equation (2), Table 4 and the procedure steps. Both c=2 panels fail, and are therefore not used as oracles:
z_{1,1} = z_{1,2} = 1. As printed the column is unbalanced 7-to-9 and the main-effects plan is not orthogonal, defeating the procedure's purpose. A unique single-entry flip restores both.The other six panels are self-consistent and reproduce exactly. Both findings are recorded as executable tests in
TestFigure1Errata, not prose.Adjacent bugs found while auditing the same paths
estimate_screening_runs(k, "definitive_screening")returned2k + 1for everyk, undercounting every odd factor count (a 7-factor DSD is 17 runs, not 15) and every stepped-up count. Now driven by the constructor, and it takesn_categorical/categorical_methodso categorical centre runs are counted too. A test had enshrined the wrong value. The recommendation engine passes the count of two-level categorical factors, exposed asDOEProblemSpec.n_two_level_categorical.fake_factorparameter that does not exist, justified by a comment with the parity argument backwards.design_type="omars"sharesdispatch_dsd, so adding categorical support silently extended it to OMARS, returning a design withomars_verified: False. OMARS is defined for quantitative factors, so it now rejects categorical ones and points atdesign_type="dsd".generate_design(..., design_type="taguchi")with a categorical factor raisedValueError: All values must be present in 'levels'.dispatch_taguchicodes categorical factors as level indices0..n-1, which reached theColumnconstructor unmapped. Fixed by the same label mapping the DSD needed.tests/test_tool_spec.py::test_registered_in_registrydepended on sibling tests having run in the same worker process, so it passed or failed depending only on how pytest-xdist split the run. Pre-existing onmain; this branch surfaced it by changing the test count._read_remote_csvconverts network failures into aRuntimeError, but the helper caught onlyURLError/HTTPError/OSError, none of which can escape the loader. A 502 from openmv.net failed a job on this branch rather than skipping._read_remote_csvnow also retries a transient failure (three attempts, 1s and 2s backoff), while a 404, 403 or parse error still raises immediately._is_primebecame unreachable when_prime_power_factorizationreplaced it. Removed.New public API
dsd_run_count(n_factors, n_categorical, categorical_method),dsd_conference_order(n_factors)anddsd_centre_runs(n_categorical, categorical_method), exported fromprocess_improve.experiments, for sizing a design without building it.Attribution for the order-16 table
Recorded in an attribute docstring on
_CONFERENCE_MATRIX_16. It originates in Xiao, Lin & Bai (2012) and reached this repository through two ports: Jacob Albrecht's 2015 MATLAB port of the JMP add-in (BSD 3-Clause, Copyright (c) 2015 Jacob Albrecht, with Bristol-Myers Squibb named in the no-endorsement clause as his affiliation, not the copyright holder), and Daniele Ongari's Python translation indefinitive_screening_design(BSD 3-Clause, Copyright (c) 2022 Daniele Ongari). Only the numeric table is reused, and it is verified rather than trusted: the order-10 table in the same upstream file has a mistyped entry that leaves that package's 9- and 10-factor designs non-orthogonal at r = 0.11.Known gap, deliberately not addressed here
The docs build has the same exposure to an openmv.net outage that the tests had, and more broadly: seven case-study notebooks call
pd.read_csv("https://openmv.net/...")directly, so neither the retry nor the skip reaches them, and nbsphinx executes each in its own kernel withnbsphinx_allow_errors = False. Closing that means either adding loaders for five more datasets and rewriting the notebooks to use them, or vendoring the CSVs. Both change the book's teaching material, so both are left for a separate decision.Test plan
tests/test_conference_matrices.py(163 tests): finite-field arithmetic checked as a field, the quadratic character checked as multiplicative and against Euler's criterion, every conference matrix checked against its defining property, the constructibility predicate cross-checked against the constructor at every order to 120, and the validator checked against a single flipped sign.tests/test_dsd_categorical.py(202 tests): the paper oracles above, plus factor-order independence, the large-c heuristic still producing a definitive design, the planner's run estimates, and the label-mapping branches.tests/test_experiments_datasets.py(18 tests): the skip guard and the retry policy, including that a settled status is not retried.test_main_effects_are_exactly_orthogonalis now unconditional (it used to skip precisely when the bug fired).budget.pyanddatasets.pyat 100%.ruff check .andmypy src/process_improveclean.Checklist
pyproject.toml(1.60.0 → 1.62.0, MINOR for the new feature),CITATION.cffin sync. 1.61.0 was taken by Compute score contributions from the data, not from the score #476 landing on main mid-review; this branch's two in-PR changelog sections were consolidated into one 1.62.0 entry, since neither intermediate version was ever tagged.ruff check .passesCHANGELOG.mdupdated