Skip to content

Model API and robustness defects found in a pre-submission audit #206

Description

@vahid-ahmadi

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:397 hardcodes q = 0.5 when fit receives no quantiles kwarg, so the cross-model idiom Model().fit(tr, X, y).predict(te, quantiles=[0.1, 0.5, 0.9]) — which works for OLS and QRF — raises RuntimeError: 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 to config.QUANTILES.
  • Matching.predict(quantiles=...) returns the same draw for every quantile. models/matching.py:337-366 copies one hot-deck match into every entry, so q10 == q50 == q90 exactly. 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_col for 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-508 returns hyperparameters=best_params, discarding matching_kwargs, which is what carries donor_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 inertfull_data=True makes utils/data.py:516-522 return before train_test_split, so 0.8 and 0.1 give bit-identical output.
  • autoimpute(random_state=...) seeds only the CV fold split, never the models, which fall back to config.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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions