Repository navigation
Model API and robustness defects found in a pre-submission audit #206
Description
A set of smaller defects found while auditing the models ahead of the JOSS submission (#201). Each is reproduced; grouping them because they are individually small but together they are most of what a reviewer exercising the API would hit.
The uniform fit/predict interface is not actually uniform
QuantReg.fit()only ever fits the median.models/quantreg.py:397hardcodesq = 0.5whenfitreceives noquantileskwarg, so the cross-model idiomModel().fit(tr, X, y).predict(te, quantiles=[0.1, 0.5, 0.9])— which works for OLS and QRF — raisesRuntimeError: Failed to access 0.1 quantile for prediction. It works only if the same quantiles are declared at fit time. Fit lazily at predict time, or default toconfig.QUANTILES.Matching.predict(quantiles=...)returns the same draw for every quantile.models/matching.py:337-366copies one hot-deck match into every entry, soq10 == q50 == q90exactly. Any quantile-loss comparison involving Matching is therefore meaningless, including its own_tune_hyperparameters, which scores at q = 0.5.
Intercept dropped on constant predictors
sm.add_constant defaults to has_constant="skip", so a single-row test frame — or any homogeneous subgroup — loses the intercept and prediction dies with ValueError: shapes (1,1) and (2,) not aligned. Affects models/ols.py:237 and models/quantreg.py:102. Fix with has_constant="add" (also worth doing at ols.py:170,180 and quantreg.py:361).
Silent all-NaN output
OLS and QuantReg log "Data contains N missing values", fit successfully, and then return nan for every prediction. QRF raises instead, which is the right behaviour. Silent corruption is worse than an error.
Weights dropped in two places
- Zero-inflated ignores
weight_colfor numeric targets.models/zero_inflated.py:182-295— weights reach only the auxiliary non-numeric imputer at line 277; the gate classifiers (335/352/372/400) and_fit_base_single(434) are unweighted. With 1000:1 weights on zero rows, weighted and unweighted output are bit-identical. - Matching drops weights when tuning.
models/matching.py:477-508returnshyperparameters=best_params, discardingmatching_kwargs, which is what carriesdonor_sample_weight. Fix:hyperparameters={**matching_kwargs, **best_params}.
Metric evaluation ignores the quantiles present in the data
comparisons/metrics.py passes the global config.QUANTILES into _compute_method_losses, so evaluating imputations produced on a custom grid raises ValueError: Quantile 0.05 not found in imputations for method m1. Derive the grid from the imputation dicts instead.
Two documented parameters with no effect
autoimpute(train_size=...)is inert —full_data=Truemakesutils/data.py:516-522return beforetrain_test_split, so0.8and0.1give bit-identical output.autoimpute(random_state=...)seeds only the CV fold split, never the models, which fall back toconfig.RANDOM_STATE. Seeds 1, 2 and 999 give different CV scores but identical final imputations — so users cannot draw independent replicates, which is the point of multiple imputation.
Tuning selects on the best held-out fold
evaluations/cross_validation.py picks hyperparameters from the fold with the lowest test loss, then reuses mean_test from that same run for model selection. That optimistically biases tuned models relative to untuned ones.
Unverified, flagged for someone with the R stack
utils/statmatch_hotdeck.py:114-135 passes donor_sample_weight to R as weight.don on StatMatch::NND.hotdeck, which has no such parameter — only RANDwNND.hotdeck does. It would fall into ... and be forwarded to the distance function. The Python-side plumbing is correct; the R-side behaviour could not be checked here.
Checked and clean
For completeness: no train/test leakage in cross-validation (instrumented row ids, zero overlap per fold), pinball loss is correctly defined, and weight_col propagates correctly to OLS and QRF. QuantReg raises an explicit NotImplementedError for weights, which is the right call.
Activity
Add an architecture figure, transpose the table, correct three false …
Add an architecture figure, transpose the table, correct three false …