Switch public uplift trees to the kernel backend, delete uplift.pyx (#955) - #973
Merged
Merged
Conversation
…955) Flip the public UpliftTreeClassifier / UpliftRandomForestClassifier to thin subclasses of the kernel-backed _KernelUpliftTreeClassifier / _KernelUpliftRandomForestClassifier (grown on the shared _tree Cython kernel), and delete the ~2.7 KLoC legacy uplift.pyx duplicate plus its import-time save/load monkey-patch. Final issue of the tree-unification epic #945. Legacy constructor names / defaults / predict semantics are preserved: evaluationFunction still selects the criterion; the tree's predict returns per-group P(Y=1|T=g) (absent-group leaf rates zero-filled, #585); fill, uplift_forest, and fitted_uplift_tree remain. early_stopping_eval_diff_scale and fit's X_val / treatment_val / y_val are accepted but ignored (the kernel has no validation-set early stopping). DecisionTree stays importable via a DeprecationWarning shim. Compatibility shims on the public subclasses: - feature_importances_ exposed non-negative (kernel split gains are signed) - int max_features clamped to n_features (#569) - IDDP forces the honest approach at construction (legacy behavior) Tests: tests/test_uplift_trees.py and tests/test_serialization_extended.py pass against the new impl; the legacy-vs-kernel parity tests and the parity notebook are removed (the public classes now ARE the kernel classes), and the remaining tests/test_uplift_trees_kernel.py cases cover kernel behavior.
jeongyoonlee
marked this pull request as ready for review
July 26, 2026 00:23
jeongyoonlee
requested review from
alexander-pv,
huigangchen,
paullo0106,
ppstacy,
ras44,
t-tte,
vincewu51 and
zhenyuz0500
July 26, 2026 00:23
10 tasks
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
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.
What
Final issue of the tree-unification epic (#945): flip the public
UpliftTreeClassifier/UpliftRandomForestClassifierto thin subclasses of the kernel-backed_KernelUpliftTreeClassifier/_KernelUpliftRandomForestClassifier(grown on the shared_treeCython kernel), and delete the ~2.7 KLoC legacyuplift.pyxduplicate plus its import-time save/load monkey-patch. Closes #955.Builds on the merged prereq #972 (native NaN support), #971 (#954 serialization +
BaseEstimatorconformance), and the rest of the epic (#946–#953).Backward compatibility (names / args / defaults / predict semantics preserved)
evaluationFunctionstill selects the criterion (KL/ED/Chi/CTS/DDP/IT/CIT/IDDP); mapped internally to the kernelcriterion..predict(X)returns per-groupP(Y=1|T=g)including control (shape(n, n_groups)), matching the historical return; absent-group leaf rates are zero-filled (uplift p_value is NAN #585)..fill(),.uplift_forest,.fitted_uplift_tree, and forest.predict(full_output=...)are all preserved.early_stopping_eval_diff_scaleandfit'sX_val/treatment_val/y_valare accepted for backward compatibility but ignored (the kernel has no validation-set early stopping) — aUserWarningis emitted when validation arrays are passed.DecisionTreestays importable via aDeprecationWarningshim.Necessary behavior shims on the public subclasses
feature_importances_exposed non-negative + renormalized (the kernel's raw importances are signed, since an uplift split gain is not a monotone impurity decrease).max_featuresclamped ton_features(legacy behavior; the kernel tree validates0 < max_features <= n_featuresstrictly) (shape mismatch when calling UpliftRandomForestClassifier.predict() #569).Removed
causalml/inference/tree/uplift.pyx(2742 lines) + itssetup.pybuild entry + thetree/__init__.pymonkey-patch.uplift_tree_kernel_parityexample notebook — obsolete now that the public classes are the kernel classes.tests/test_uplift_trees_kernel.pyretains the kernel behavioral tests (criteria, regularization / normalization / honesty effects, pruning, plot, serialization, missing-value).Verification
tests/test_uplift_trees.py— 27 passed (acceptance; the onenodeSummaryassertion migrated to the kernel's per-node group counts per the epic plan).tests/test_serialization_extended.py— passed.tests/test_uplift_trees_kernel.py— 48 passed (trimmed to kernel behavior).tests/test_causal_trees.py— passed (regression).blackclean.Notes
_uplift/package was intentionally not renamed touplift/(the issue lists it as optional) to keep the diff surgical.check_estimatorremains N/A (upliftfitis(X, treatment, y), not(X, y)), consistent with [Tree unification 8/9] Serialization + BaseEstimator + check_estimator; drop the monkey-patch #954's curated-conformance decision and documented in the class docstrings.Part of #945.