Skip to content

Add tidypredict_combine_trees() and the last extractors - #436

Open
EmilHvitfeldt wants to merge 1 commit into
mainfrom
missing-extractors
Open

Add tidypredict_combine_trees() and the last extractors#436
EmilHvitfeldt wants to merge 1 commit into
mainfrom
missing-extractors

Conversation

@EmilHvitfeldt

Copy link
Copy Markdown
Member

T4 / phase 1d, and the last piece of the orbital new-backends plan's tidypredict-side work.

The plan for this PR was wrong, in a way the code made obvious

Phase 1a claimed that "any backend that gains a tidypredict_trees() method automatically gains separate_trees in orbital with no orbital change at all." That only holds when a model combines its trees by a plain sum or mean. Checking the five backends this PR was supposed to cover, it holds for one:

Backend Trees? How the model actually combines them
cforest yes plain mean ✓
aorsf yes mean, inside a guard returning NA for a partial row ✗
blackboost yes offset + nu * sum(trees)
C5.0 yes confidence-weighted vote over class labels, not summable ✗
xrf none penalized glm on rule indicators, not an ensemble

Adding the methods as specified would have let a caller drop blackboost's offset and shrinkage, and aorsf's missing-row guard: plausible wrong numbers rather than an error.

tidypredict_combine_trees(x, trees)

Holds the combination rule alongside the trees. trees may be expressions or symbols naming columns the trees were written to, because computing each tree into its own column and then combining references to those columns is the case this exists for.

The rules, all of which a caller would otherwise have to know:

blackboost    offset + nu * sum(trees)
aorsf         mean(trees), inside an NA guard
catboost      scale * sum(trees) + bias, then an inverse link
xgboost       base_score + sum(trees), then the objective's inverse link
lightgbm      sum(trees), divided by n for boosting = "rf", then the link
randomForest  mean(trees), plus corr.bias rescaling when present
cforest       mean(trees)

The identity is enforced structurally. Every ensemble's tidypredict_fit() now routes through the two generics, so

tidypredict_combine_trees(x, tidypredict_trees(x)) == tidypredict_fit(x)

holds by construction rather than by coincidence. The arithmetic was factored into one helper per backend (rf_combine(), ranger_combine(), xgb_combine(), lgb_combine(), catboost_combine(), mboost_combine()) called from both paths, so it cannot drift. Tests assert the identity at the value level for all nine backends, and again with rlang::syms() standing in for tree columns. It held on the first attempt for every backend, including DART and binary:logistic.

The xgboost helper is arranged so double-applying DART weights is structurally impossible: assemble_xgb_formula() is now xgb_combine(apply_dart_weights(...)).

Two backends deliberately expose nothing

Boosted C5.0 combines by a confidence-weighted vote where each trial contributes a class label plus a confidence, with SelectClassGen() breaking ties on that trial's own root class. There are no per-tree numbers, so splitting the trees apart is not an approximation of the model, it is a different model. tidypredict_combine_trees() refuses with an explanation rather than a bare "no method".

xrf is not a tree ensemble at all. It is a penalized linear model whose predictors are rule indicators, built as a terms list of coefficient times indicator, and it already works as a pm_regression. The plan listed it here on a false premise.

Multiclass lightgbm and catboost also refuse: their tidypredict_trees() output is a flat round-robin list whose adjacent trees belong to different classes, so it is not summable as a single ensemble.

A comment of mine from #433 was wrong

tidypredict_n_trees.lgb.Booster() claimed single-leaf trees are dropped by the extractor. They are not: lgb.model.dt.tree() omits them but add_lgb_stump_trees() deliberately puts them back, because multiclass class assignment is positional and a gap shifts every later class (#419). Comment corrected.

Verification

Suite FAIL 0 | WARN 6 | SKIP 1 | PASS 2703, up from 2675. R CMD check with vignettes: 0 errors, 0 warnings.

One thing worth flagging about the tests. The new ones initially passed under devtools::test() and failed under R CMD check with could not find function "case_when". Tree expressions call case_when() unqualified, and tests/testthat.R attaches only testthat and tidypredict — they were passing only because an earlier test file had attached dplyr, which check does not reproduce. Fixed by evaluating against dplyr's namespace explicitly, so the tests no longer depend on run order.

Four pre-existing snapshots changed only in the reported call label, since the objective checks moved into the extracted helpers. Messages are byte-identical and the new labels are accurate.

Noted, not fixed

  • apply_xgb_objective() tests base_score != 0 on a value get_xgb_json_params() can parse as length > 1, since its regex splits on commas. Unreachable today because multiclass objectives are rejected, but a latent trap.

tidypredict_trees() returns per-tree expressions, but putting them back
together is not summing or averaging them, and the rule differs by
backend:

  blackboost   offset + nu * sum(trees)
  aorsf        mean(trees), inside a guard returning NA for a partial row
  catboost     scale * sum(trees) + bias, then an inverse link
  xgboost      base_score + sum(trees), then the objective's inverse link
  lightgbm     sum(trees), divided by n for boosting = "rf", then the link
  randomForest mean(trees), plus corr.bias rescaling when present
  cforest      mean(trees)

So exposing the trees alone was not enough. A caller doing the obvious
thing would drop blackboost's offset and shrinkage, and aorsf's missing-
row guard, and get plausible wrong numbers rather than an error.

Add tidypredict_combine_trees(x, trees), which holds that rule. `trees`
may be expressions or symbols naming columns the trees were written to,
since computing each tree into its own column and then combining
references to them is the case this exists for.

Every ensemble's tidypredict_fit() now routes through the two generics,
so the identity

  tidypredict_combine_trees(x, tidypredict_trees(x)) == tidypredict_fit(x)

holds by construction rather than by coincidence. The tests assert it at
the value level for all nine backends, and again via symbols.

Also adds tidypredict_trees() and tidypredict_n_trees() for cforest,
blackboost and aorsf.

Two backends deliberately have no trees exposed. Boosted C5.0 combines
by a confidence-weighted vote over class labels, so there are no per-tree
numbers to combine at all; tidypredict_combine_trees() refuses with an
explanation rather than a bare "no method". And xrf is not a tree
ensemble: it is a penalized linear model whose predictors are rule
indicators, already handled as a pm_regression.

Multiclass lightgbm and catboost also refuse: their tidypredict_trees()
output is a flat round-robin list whose adjacent trees belong to
different classes, so it is not summable as one ensemble.

Corrects a stale comment on tidypredict_n_trees.lgb.Booster() claiming
single-leaf trees are dropped by the extractor. add_lgb_stump_trees()
deliberately restores them, because multiclass class assignment is
positional and a gap shifts every later class (#419).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant