Skip to content

Add conservative dependency lower bounds and drop now-dead sklearn compat branches - #1049

Open
kbattocchi wants to merge 3 commits into
mainfrom
kebatt/raise-scipy-floor
Open

Add conservative dependency lower bounds and drop now-dead sklearn compat branches#1049
kbattocchi wants to merge 3 commits into
mainfrom
kebatt/raise-scipy-floor

Conversation

@kbattocchi

@kbattocchi kbattocchi commented Jul 30, 2026

Copy link
Copy Markdown
Member

Fixes the nightly (floating-dependency) failures on Python 3.12, and hardens dependency resolution generally by giving every dependency a real lower bound.

What broke

Nightly runs started failing on Python 3.12 on 2026-07-29 — the day after uv 0.12.0 was released (2026-07-28). The 2026-07-28 nightly used uv 0.11.32 and passed:

nightly uv Run tests (ubuntu-latest, 3.12, main)
07-28 0.11.32 ✅ scipy 1.15.3, numba 0.66.0
07-29 0.12.0 Building scipy==1.6.1 → build failure

The error:

Building scipy==1.6.1
  x Failed to build `scipy==1.6.1`
       ModuleNotFoundError: No module named 'distutils' hint: `distutils` was removed from the standard library in Python 3.12.
       Consider adding a constraint (like `scipy >1.6.1`) ...

Root cause

Under uv 0.12.0's default prerelease handling the resolver selects the prerelease numba 0.67.0rc1 — currently the only numba permitting numpy >= 2.5 — which pulls in numpy 2.5.1 and then back-tracks the rest of the graph. Because scipy > 1.4.0 placed essentially no floor on scipy, the resolver was free to settle on scipy 1.6.1 (Feb 2021), which has no wheels for Python 3.12 and fails to build from source. Reproduced locally with uv 0.12.0 (my system uv 0.10.0 could not reproduce it, which is why the uv version turned out to be the key variable):

--prerelease scipy numba numpy
(default) 1.6.1 0.67.0rc1 2.5.1
if-necessary 1.6.1 0.67.0rc1 2.5.1
disallow 1.15.3 0.66.0 2.4.6

Notably --prerelease if-necessary does not help.

Why not just pass --prerelease disallow in CI

That would only protect our own CI. Any user installing econml with uv 0.12+ can hit the same resolution. The declarative fix — which is also what uv's own hint suggests — is to give the dependencies real lower bounds.

The change

Bounds are set to roughly the newest release available at the end of 2024, capped so they never exceed the version actually exercised on our oldest supported interpreter (Python 3.9) in lkg.txt. That cap is what binds numpy (2.0.2), scipy (1.13.1) and matplotlib (3.9.4); the rest take their 2024 value. Six runtime dependencies previously had no lower bound at all:

before after before after
numpy (none) >= 2.0.2 pandas > 1.0 >= 2.2.3
numba > 0.53.1 >= 0.60.0 shap >= 0.38.1 >= 0.46.0
scipy > 1.4.0 >= 1.13.1 lightgbm (none) >= 4.5.0
scikit-learn >= 1.0 >= 1.6.0 packaging (none) >= 24.2
sparse (none) >= 0.15.4 graphviz (none) >= 0.20.3
joblib >= 0.13.0 >= 1.4.2 matplotlib (none) >= 3.9.4
statsmodels >= 0.10 >= 0.14.4 ray > 2.2.0 >= 2.40.0

[build-system].requires had the same gap and is fixed too: setuptools >= 75.6.0, wheel >= 0.45.1, scipy >= 1.13.1, cython >= 3.0.11.

Verification

With uv 0.12.0 and no flags, all 18 combinations of Python 3.9-3.14 x {[plt], [plt,dowhy], [plt,dowhy,ray]} resolve successfully, and the resolved versions still match lkg.txt:

py3.9  [plt,dowhy,ray]   scipy=1.13.1  numba=0.60.0  numpy=2.0.2  sklearn=1.6.1
py3.10 [plt,dowhy,ray]   scipy=1.15.3  numba=0.66.0  numpy=2.2.6  sklearn=1.7.2
py3.12 [plt,dowhy]       scipy=1.15.3  numba=0.66.0  numpy=2.4.6  sklearn=1.9.0
py3.14 [plt,dowhy,ray]   scipy=1.18.0  numba=0.66.0  numpy=2.4.6  sklearn=1.9.0

Python 3.9 continues to resolve exactly what LKG pins. Also confirmed the package builds, installs and imports cleanly.


Update: dead-branch cleanup folded in

Now that the scikit-learn floor moves to >= 1.6.0 by the same end-of-2024 rule used for every other dependency, a second commit removes the compatibility branches that become unreachable at that floor.

SKLEARN_GE_12, SKLEARN_GE_14, SKLEARN_GE_15 and SKLEARN_GE_16 are all unconditionally true at the new floor and are deleted; only SKLEARN_GE_17 and SKLEARN_GE_18 still distinguish supported versions. (SKLEARN_GE_16 is dead precisely because the floor is now 1.6.)

  • _sklearn_compat.py — the two 1.5 symbol re-exports become plain imports (hoisted to the top import block so they don't trip E402); one_hot_encoder always passes sparse_output; ensure_finite_kwargs always returns ensure_all_finite
  • model_selection.py_cross_val_predict drops the pre-1.4 call that still passed verbose to sklearn's non-public _fit_and_predict
  • tests/test_sklearn_compat.py — assertions that branched on the same flags become unconditional

Docs

The README compatibility table still documents these change kinds, since they remain good worked examples. Instead of deleting or rewording them, the citations now point at commit-SHA permalinks of the pre-cleanup code plus a note that each branch was removed at the 1.6 floor. They are pinned to a SHA rather than a release tag because _sklearn_compat.py postdates v0.16.0 and is not in any tag yet, so a tag link would 404.

kbattocchi and others added 2 commits July 30, 2026 13:57
Nightly runs installing floating dependencies began failing on Python 3.12
on 2026-07-29, the day after uv 0.12.0 was released (2026-07-28; the prior
nightly used uv 0.11.32 and passed). Under uv 0.12.0's default prerelease
handling the resolver selects the prerelease numba 0.67.0rc1 - currently the
only numba permitting numpy >= 2.5 - which pulls in numpy 2.5.1 and then
back-tracks the rest of the graph. Because "scipy > 1.4.0" placed almost no
floor on scipy, the resolver was free to settle on scipy 1.6.1, which has no
wheels for Python 3.12 and fails to build from source since distutils was
removed from the standard library:

    ModuleNotFoundError: No module named 'distutils'

Passing --prerelease disallow avoids it, but that would only protect CI;
any user installing econml with uv 0.12+ can hit the same resolution. The
declarative fix is to give every dependency a real lower bound.

Bounds are set to roughly the newest release available at the end of 2024,
capped so they never exceed the version actually exercised on our oldest
supported interpreter (Python 3.9) in lkg.txt. numpy (2.0.2), scipy (1.13.1)
and matplotlib (3.9.4) are capped by that rule; the rest take their 2024
value. Several dependencies (numpy, sparse, lightgbm, packaging, graphviz,
matplotlib) previously had no lower bound at all, as did the build-system
requirements for scipy, cython, setuptools and wheel.

Also raises scikit-learn from >= 1.0 to >= 1.6.0. We have never tested below
1.6.1 (the lowest pin in lkg.txt), so the old floor claimed support we do not
verify. This covers the pyproject half of the pending "raise sklearn floor"
work; removing the now-dead SKLEARN_GE_12/14/15 branches remains outstanding.

Verified with uv 0.12.0 using default settings (no --prerelease flag) that
all 18 combinations of Python 3.9-3.14 x {[plt], [plt,dowhy], [plt,dowhy,ray]}
resolve successfully, and that the resolved versions still match lkg.txt.
Python 3.9 continues to resolve numpy 2.0.2 / scipy 1.13.1 / scikit-learn
1.6.1. Also confirmed the package builds, installs and imports.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Keith Battocchi <kebatt@microsoft.com>
Raising the declared scikit-learn floor to >= 1.6.0 in the previous commit
makes several version gates unconditional, so the branches they guard are
now dead code.

SKLEARN_GE_12, SKLEARN_GE_14, SKLEARN_GE_15 and SKLEARN_GE_16 are always
true at the new floor and are removed; only SKLEARN_GE_17 and SKLEARN_GE_18
still distinguish supported versions. Note this includes SKLEARN_GE_16,
which is dead precisely because the new floor is 1.6.

Collapsed accordingly:

* _sklearn_compat.py: the two symbol re-exports (_get_column_indices,
  _print_elapsed_time, both relocated in 1.5) become plain imports, hoisted
  into the top import block so they no longer trip E402; one_hot_encoder
  always passes sparse_output (renamed in 1.2); ensure_finite_kwargs always
  returns ensure_all_finite (renamed in 1.6).
* model_selection.py: _cross_val_predict drops the pre-1.4 call that still
  passed `verbose` to sklearn's non-public _fit_and_predict.
* tests/test_sklearn_compat.py: the helper assertions were branching on the
  same flags, so they are now unconditional.

The README compatibility table keeps documenting these change kinds, since
they remain useful worked examples of renamed kwargs and relocated symbols.
Rather than delete or reword them, the citations now point at commit-SHA
permalinks of the pre-cleanup code, with a note that each branch was removed
once the floor reached 1.6. Permalinks are pinned to a SHA rather than a
release tag because _sklearn_compat.py postdates v0.16.0 and so does not
exist in any tag yet.

Verified with ruff over the whole package, the compat and compat-helper test
modules, the linear_model clone/set_params/n_alphas/future-warning tests, and
the full test_model_selection module (which exercises the changed
_cross_val_predict path) against scikit-learn 1.9.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Keith Battocchi <kebatt@microsoft.com>
@kbattocchi kbattocchi changed the title Add conservative lower bounds to all dependencies (fixes nightly Python 3.12 resolution failure) Add conservative dependency lower bounds and drop now-dead sklearn compat branches Jul 30, 2026
@kbattocchi
kbattocchi requested a review from Copilot July 31, 2026 14:56

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens EconML’s dependency resolution (notably under uv on Python 3.12+) by introducing conservative lower bounds for runtime/build dependencies, and simplifies scikit-learn compatibility code by removing now-unreachable version branches after raising the sklearn floor.

Changes:

  • Add explicit conservative lower bounds for core and optional dependencies (plus build-system requirements) in pyproject.toml.
  • Remove dead scikit-learn compat branches/flags and simplify call sites/tests accordingly.
  • Update README scikit-learn compatibility documentation to reflect the new flags and link to historical examples via commit permalinks.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
README.md Updates sklearn-compat documentation/examples to match the new supported range and removed branches.
pyproject.toml Adds conservative lower bounds for runtime/optional deps and for build-system requirements to prevent resolver backtracking.
econml/tests/test_sklearn_compat.py Removes branching assertions now that sklearn <1.6 is unsupported.
econml/sklearn_extensions/model_selection.py Drops the pre-1.4 _fit_and_predict call signature branch.
econml/_sklearn_compat.py Removes obsolete version flags and conditional imports; makes shims unconditional given the new sklearn floor.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@kbattocchi
kbattocchi requested a review from fverac July 31, 2026 15:01
@kbattocchi
kbattocchi marked this pull request as ready for review July 31, 2026 15:01
The ForestDRIV example was passing or failing depending on which machine
in the GitHub Actions fleet ran the docs job. Two runs of the identical
commit, with byte-identical resolved package sets (218 packages, zero
version differences) and the same runner image, produced:

    hi = array([ 1.36811, 10.23568, -0.16722])   # passes
    hi = array([ 1.37008, 10.23568, -0.17805])   # fails

Both values are legitimate. The difference is reproducible locally by
changing only the OpenBLAS kernel: OPENBLAS_CORETYPE=Nehalem yields
-0.1672216 (matching the recorded expectation) while the default AVX2
path yields -0.17805291. Runner images are identical across the fleet,
but the underlying CPUs are not, so OpenBLAS picks different kernels.

Two things stack to make this example uniquely fragile:

- DRIV pseudo-outcomes divide by an estimated compliance rate, and this
  DGP uses a deliberately weak instrument, so the final forest sees noisy
  targets with many near-tied candidate splits. Comparing the fitted
  forest across the two kernels, 9 of 58704 nodes differ in split feature
  and threshold; those flipped splits change honest leaf assignments and
  move predictions by ~0.1%.
- The recorded upper bound sits near zero, so scipy-doctest's rtol=0.01
  allows only +/-0.0017 on it -- roughly 40x tighter in absolute terms
  than any other value in the doctest suite. The actual movement is 0.011,
  i.e. 0.16% of the interval's width.

Nothing is wrong with the estimator. A census of all 101 numeric doctest
statements under both kernels found 78 bit-identical, 21 drifting under
0.01%, and only this example exceeding rtol -- CausalForestDML and
grf.RegressionForest are bit-identical, so this is not a general property
of the forest estimators.

Shift the example's ground-truth effect by a constant baseline so no
printed value sits near zero. Reducing n_estimators would also stabilize
it, but doctests need to stay fast and the default is what we want to
document. With the offset, the smallest printed value is 3.03, so rtol
allows +/-0.03 against observed drift of ~0.01, and the raw outputs are
bit-identical across both kernels.

Verified with scipy-doctest's DTChecker under both OPENBLAS_CORETYPE
settings: 2 examples attempted, 0 failed.

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

kbattocchi commented Jul 31, 2026

Copy link
Copy Markdown
Member Author

Pushed 92dec46 to fix the intermittent ForestDRIV doctest failure that was making this PR red. Summary of the investigation, since the fix is slightly outside this PR's stated scope.

The change here is not the cause. Three independent checks:

  • d0d7f682 reverted to b4446f0a's versions of the three touched files produces bit-identical output locally. Every branch it removed was the else of a SKLEARN_GE_1x flag for x <= 16, and CI runs sklearn 1.8.0, so it is a no-op by construction as well as empirically.
  • The passing and failing docs jobs resolved byte-identical environments — 218 packages, zero version differences — on the same runner image, from the same merge base.
  • The same commit subsequently failed and then passed on re-run.

Root cause: OpenBLAS kernel selection. Reproducible locally by changing nothing but OPENBLAS_CORETYPE:

coretype est.effect_interval(X[:3])[1]
Nehalem [ 1.368358, 10.235683, -0.167222] (matches the recorded expectation)
default / Haswell [ 1.369840, 10.235683, -0.178053]

Runner images are identical across the fleet; the CPUs behind them are not.

Why only this doctest. Two amplifications stack. DRIV pseudo-outcomes divide by an estimated compliance rate and this DGP uses a deliberately weak instrument, so the final forest sees noisy targets with many near-tied splits — comparing the fitted forests across the two kernels, 9 of 58704 nodes differ in split feature and threshold, and those flipped splits change honest leaf assignments. On top of that the recorded upper bound sits near zero, so rtol=0.01 allows only +/-0.0017 on it, roughly 40x tighter in absolute terms than anything else in the suite. The actual movement is 0.011, i.e. 0.16% of the interval's width.

Nothing systemic is wrong. I ran a census of all 101 numeric doctest statements under both kernels:

78  bit-identical
21  drift < 0.01%
 1  drift 0.13%   (ForestDRIV est.effect)
 1  drift 6.08%   (ForestDRIV est.effect_interval)  <- the only one exceeding rtol

CausalForestDML and grf.RegressionForest are bit-identical, so this is not a general property of the forest estimators.

The fix shifts the example's ground-truth effect by a constant baseline so no printed value sits near zero. The smallest printed value is now 3.03, so rtol allows +/-0.03 against observed drift of ~0.01, and the raw outputs are bit-identical across both kernels. Verified with scipy-doctest's DTChecker under both settings: 2 examples attempted, 0 failed. Raising n_estimators also stabilizes it, but doctests should stay fast and the default is what we want to document.

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.

2 participants