Repository navigation
0.9.1: refuse missing values instead of mis-compiling them - #80
Merged
Merged
Conversation
…t reproduce The histogram backend trains on NaN and learns which way missing values go at every split; the artifact has no missing-value branch and imputes the baseline instead, so the two scored those rows differently without a word. compile_selected also took the baseline as np.median, which is NaN for any column with a missing value, and shipped it: invalid JSON, and NaN routed through tree comparisons, which spec section 8 forbids. train_whitebox and compile_selected now refuse NaN in X, naming the columns and the fix (impute, add a 0/1 missing-indicator column). build_artifact refuses a non-finite baseline or split threshold. Extraction refuses a histogram-boosting model whose splits show it saw NaN: without NaN at fit time scikit-learn sends missing values to the larger child, so a direction that contradicts the child counts, or a split at infinity, can only come from training on NaN. The old note fired on nearly every model and proved nothing, so it is gone. Checked on 30 models trained without NaN (weighted, monotone, early stopping): no false positives. The duplicate check's message now names both causes: repeat entities (group=) and rows identical by construction (raise duplicate_threshold).
Missing values are refused instead of mis-compiled. A patch release: no change to the runtime, the exporters, the artifact schema, or any artifact built from data without missing values. The benchmark does not move, so results.json still records the 0.9.0 run that produced it.
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
1 participant
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.
A patch: missing values are refused instead of mis-compiled. Nothing changes for data without missing values: not the runtime, the exporters, the artifact schema, or the benchmark.
The bug
The histogram backend trains on NaN and learns which way missing values go at every split. Monotone constraints have used it since 0.2.0, and
compile_selecteduses it by default. The artifact has no missing-value branch and imputes the baseline instead. Reproduced with 20% missing values in one feature: the model scores a missing row at 0.784, the artifact at 0.865, and nothing warns.compile_selectedmade it worse. It took the baseline asnp.median, which is NaN for any column with a missing value, and shipped it. The result was invalid JSON, with NaN routed through tree comparisons, which spec §8 forbids.The fix
train_whiteboxXon every backend, naming the columns and the fix: impute, and add a 0/1 missing-indicator columncompile_selectedbuild_artifactAbout the extraction check. HGB records a missing-value direction on every split, learned or not, so the old note fired on nearly every model and proved nothing. Without NaN at fit time, scikit-learn sends missing values to the larger child. A direction that contradicts the child counts, or a split at infinity, can therefore only come from training on NaN. On 30 models trained without NaN (weighted, monotone, early stopping) it flagged nothing. On models trained with NaN it flagged 8–50 splits per model.
Also: the duplicate check's message named only repeat applicants (
group=). It now also covers rows identical by construction, such as a no-record code in every feature. There nothing leaks, and the fix is to raiseduplicate_threshold.Release
The second commit bumps to 0.9.1 (
__init__, CITATION.cff, CHANGELOG).benchmarks/results.jsonstill says"compileml": "0.9.0". That is deliberate: it records the run that produced it, and 0.9.1 doesn't change the numbers.Checks
tests/test_missing_values.py(12 tests); full suite green locallymkdocs build --strictclean