Skip to content

[ML] Skip missing values in fixed candidate splits - #3240

Open
HaohanTsao wants to merge 1 commit into
elastic:mainfrom
HaohanTsao:fix/fixed-candidate-splits-missing-values
Open

HaohanTsao wants to merge 1 commit into
elastic:mainfrom
HaohanTsao:fix/fixed-candidate-splits-missing-values

Conversation

@HaohanTsao

@HaohanTsao HaohanTsao commented Oct 9, 2026 •

Copy link
Copy Markdown

Closes #3241

This PR goes with elastic/elasticsearch#161539. Please merge that PR first, or at the same time as this one. If this PR is merged first, analytics jobs will give normal feature importance for the affected models, but inference in Elasticsearch will still give NaN.

The problem

If a feature has only a few distinct values and some missing values, boosted tree training can build a broken model, and the job does not report any error.

  • Training. CBoostedTreeImpl::initializeFixedCandidateSplits collects the distinct values of each feature to build the candidate splits. It did not skip missing values (NaN). NaN is never equal to itself, so every missing row was added as a new value. If a feature still had 75 values or fewer, it used the fixed-candidate path. There, NaN broke std::sort, and some candidate splits became NaN.
  • The model. For some splits, the statistics showed rows on both sides, but in fact all rows went to one side. In one test (six integer features, 10 distinct values each, 2% missing values), about 93% of the splits had a child with no samples. Test accuracy was about 0.5, the same as guessing. The job still finished with no error.
  • Native TreeSHAP. At internal nodes with no samples, it computed 0 / 0. The output was importance 0 and a baseline of 0.0.

Changes

In initializeFixedCandidateSplits:

  • Skip missing values when collecting the distinct values. The check is inside the existing limit check, so encoding still stops when a feature goes over the limit.
  • Use the fixed-candidate path only if a feature has at least two distinct values. Without this check, a feature where every value is missing has no values left, and reserve(values.size() - 1) throws std::length_error. This can happen when the encoding was computed on a different data set.
  • Encode each row once, not once for every feature. refreshSplitsCache already does this.

In CTreeShapFeatureImportance: handle nodes with no samples. Models trained before this fix still have these nodes. Incremental training can also create them in a normal way, because it counts the samples of old trees again on new data. I reproduced a model with 1,184 such nodes.

  • In shapRecursive, if a node below the root has no samples, its two children each get half of the fraction.
  • In computeInternalNodeValues, such a node gets the simple average of its two children.
  • Why half and half: any split that adds up to 1 keeps the expected value and local accuracy. The split only changes how importance is shared between features, for rows that a subset of their features sends into this node. No rows reached the node, so there is no reason to prefer one child. Half and half is also the only fixed split that does not depend on which child is left and which is right.
  • The root is not changed. If the root has no samples, no rows were counted at all, so the result is still NaN.
  • Both changes look at the node's own sample count. So if the counts in a tree do not match, the result is still NaN, and not a baseline that looks correct but is wrong.
  • This is the same rule as the Java code (TreeInferenceModel) in [ML] Fix NaN feature importance for trees with zero-sample nodes elasticsearch#161539.
  • The child fraction is now computed as incoming * (child / node), in the same order as the Java code. The existing SHAP tests pass with no changes, at their 1e-7 tolerance.
  • The header comment of computeInternalNodeValues explains these cases.

Testing

New unit tests. I ran each test first on code where it should fail, and checked that it failed for the expected reason.

CBoostedTreeTest

  • testLowCardinalityFeaturesFixedCandidateSplitsWithMissingValues
    • Checks: the candidate splits are exactly 0.5, 1.5, ..., 8.5.
    • Without the fix: 19 candidate splits instead of 9.
  • testLowCardinalityFeaturesWithMissingValues
    • Checks: no nodes with zero samples after training, and R² > 0.9 (0.964 in my run).
    • Without the fix: 3534 nodes with zero samples.
  • testLowCardinalityFeatureAllMissingAfterSeparateEncoding
    • Checks: a feature chosen by an encoding from another data set, with no values in the training data.
    • Without the two-value check: std::length_error. This test also passes on the original code.

CTreeShapFeatureImportanceTest

  • testZeroSampleInnerNodeExpectedNodeValues
    • Checks: the node values and the baseline.
    • Without the fix: NaN.
  • testZeroSampleInnerNodeShap
    • Checks: Shapley values computed by hand, which add up to prediction minus baseline. One row never goes into the zero-sample node, but a subset of its features sends it there. For rows that were counted in training, this is the only way they are affected.
    • Without the fix: NaN for every row. With a 0.4/0.6 split instead of 0.5/0.5, that one row fails (5.45 instead of 5.5).
  • testZeroSampleRootIsNotMasked
    • Checks: a root with no samples still gives NaN.
    • If the root is not excluded: normal-looking numbers.
  • testInconsistentSampleCountsAreNotMasked
    • Checks: a node that has samples, but whose children have none, still gives NaN.
    • If the code checks the children's sum instead of the node's own count: a baseline of 14.

Test suites (local run, macOS arm64, Apple clang 17, Boost 1.86):

  • ml_test_maths_analytics: 126/127 passed, 1 skipped. The skipped test is testMultinomialLogisticRegressionForManyClasses, which is disabled. Before the change: 119/120, with the same skip.
  • ml_test_api: 225/226 passed. CFieldDataCategorizerTest/testJobKilledReverseSearch fails in the same way on the original code in my environment, so it is not related.
  • clang-format 5.0.1 (in the ml-check-style:2 image) shows no changes.

End to end: Elasticsearch main with my local build of data_frame_analyzer. The builds before and after the fix use the same toolchain.

Missing values Before After
0% — exactly the same as before
2%, fixed hyperparameters 93% zero-sample splits, accuracy 0.49–0.50 no zero-sample splits, accuracy 0.825–0.866
2%, default hyperparameters accuracy 0.49–0.60 accuracy 0.845–0.876
20%, 10 seeds average accuracy 0.792 (quantile path) average accuracy 0.790 (fixed-candidate path)

At 20%, the standard error is about 0.006, so this difference is just noise.

  • n_gram_encoding classification on domain names: zero-sample splits went from 1.2%, 13.3% and 34.3% down to 0% for three seeds. Native and Java feature importance match within 3e-15.
  • A model trained before the fix, with zero-sample internal nodes: the native baseline is now 0.2895 (it was 0.0). Native importance keeps local accuracy within 4.7e-7.

This branch has not been deployed

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[ML] Missing values break fixed candidate splits for low-cardinality features, silently degrading models

2 participants