Refresh LKG and record the sklearn 1.9 forest behavior change - #1052
Closed
kbattocchi wants to merge 2 commits into
Closed
Refresh LKG and record the sklearn 1.9 forest behavior change#1052kbattocchi wants to merge 2 commits into
kbattocchi wants to merge 2 commits into
Conversation
Member
Author
|
Heads-up on CI coverage for this PR: on:
pull_request:
branches:
- mainWhen using the new stacked PR feature on GitHub, this should be okay (because it treats each PR in the stack as if it targets the base of the bottom PR) but we'll have to see whether there are any catches (e.g. I created this stack using the web interface after the fact, so maybe CI won't trigger until there's another push to this PR). In the future we may want to consider further changes in light of the guidance here, in particular around not running our full matrix for each PR in the stack each time a PR is merged. |
Regenerates lkg.txt from the nightly's lkg-files artifact (run 30965815878,
2026-08-05), produced by the repo's own generate_lkg.py. The file is dropped in
verbatim; nothing is hand-edited.
The headline bump is scikit-learn 1.8.0 -> 1.9.0 on Python >= 3.11, which is what
the docs job runs. Also numpy 2.4.4 -> 2.4.6, pandas 3.0.2 -> 3.0.5, scipy
1.17.1 -> 1.18.0 (py >= 3.13), numba 0.65 -> 0.66, shap 0.51 -> 0.52 (py >= 3.12),
lightgbm 4.6 -> 4.7 (py >= 3.10), matplotlib 3.11.1 (py >= 3.11), ray 2.54.1 ->
2.56.1, Cython 3.2.4 -> 3.2.9, plus narwhals, which sklearn 1.9 adds as a
dependency of its own. pyproject already caps scikit-learn below 1.10, so no
constraint needed changing.
lkg-notebook.txt is deliberately NOT refreshed. Its regenerated form is
unsatisfiable: the two notebook cells differ only by extras ([plt,dowhy] for
customer-scenarios versus [plt,ray] for the rest), and on this run they resolved
different scipy versions, because dowhy 0.14 holds scipy at 1.15.3 while the ray
cell floats to 1.18.0. Both cells are ubuntu-latest on Python 3.12, so there is no
OS or python_version marker that can separate them, and generate_lkg.py emits both
pins unconditionally:
scipy==1.15.3
scipy==1.18.0
which fails immediately with "Because you require scipy==1.15.3 and scipy==1.18.0".
This is not really a flaw in generate_lkg.py's implementation: PEP 508 markers
cannot express which extras were requested of the project being installed, so a
single merged requirements file cannot represent two cells that differ only by
extras. Keeping the previous lkg-notebook.txt leaves the notebook jobs on their
existing pins, which is what they use today, and keeps this change focused on the
docs job, which reads lkg.txt.
One expected value moves. sklearn 1.9 changed RandomForest and ExtraTrees to use
sample_weight when drawing the bootstrap sample, rather than forwarding the
weights alongside a uniform mask. NonParamDML is maximally exposed to that,
because its final-stage weights are literally T_res**2 (see dml.py) and so are
highly skewed. Its doctest goes
array([0.35318, 1.28760, 0.83506]) -> array([0.22215931, 1.6484125 , 0.9278963 ])
This is recorded rather than worked around. Benchmarked previously on a known
DGP over 6 seeds at n=2000, mean MSE improves from 0.1795 to 0.1571 and is better
on every seed, with bias essentially unchanged, so 1.9 is the better estimator
here. econml's own forests (CausalForestDML, econml.grf, econml.policy) are
Cython and are unaffected.
Verified with the real docs-job command under Python 3.12 with the refreshed
lkg.txt, i.e. sklearn 1.9.0: 290 tests, 0 failures, and again under
OPENBLAS_CORETYPE=Nehalem to catch values sensitive to BLAS kernel selection,
also 290 tests, 0 failures.
Before the preceding ellipsis cleanup this refresh would have broken 8 doctests.
Seven of those were drift between 6e-5 and 0.8% that only failed because an
ellipsis forced literal string comparison; with those gone, the refresh touches
exactly the one value that reflects a real change in behavior.
Signed-off-by: Keith Battocchi <kebatt@microsoft.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: f11774e2-02be-42fa-a2b9-429871322083
kbattocchi
force-pushed
the
kebatt/lkg-refresh-sklearn19
branch
from
August 5, 2026 23:50
d6d93db to
28af723
Compare
Empty commit to test whether a push re-triggers CI on a stacked pull request, whose base branch is not main. Signed-off-by: Keith Battocchi <kebatt@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Member
Author
|
Closing in favor of a new approach in #1055; I'll grab the doctest updates from here and include them in a commit there. |
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.
Stacked on #1051 — base is
kebatt/doctest-drop-ellipses, so review that first.This updates
lkg.txtto use scikit-learn 1.9 (from 1.8), and updates one doctest that needs to change as a result. The updated lkg.txt is the one generated from yesterday's nightly run.The reason to address this now is that our nightlies are failing due to an sklearn change in 1.9 that changes some values in doctests; this change is actually an improvement: sklearn 1.9 changed
RandomForest/ExtraTreesto usesample_weightwhen drawing the bootstrap sample, rather than forwarding the weights alongside a uniform mask.NonParamDMLis maximally exposed, because its final-stage weights are literallyT_res**2and therefore highly skewed, so we need to update one array in our doctests: