Finish the scipy-doctest migration by dropping numeric ellipses - #1051
Open
kbattocchi wants to merge 1 commit into
Open
kbattocchi wants to merge 1 commit into
kbattocchi wants to merge 1 commit into
Conversation
doc/conf.py monkey-patches scipy-doctest's DTChecker as doctest.OutputChecker so that float output is compared numerically (rtol=0.01, atol=1e-8) rather than as text. The migration to it was deliberately partial: 365 numeric ellipses across 143 lines were left in place. Those ellipses actively defeat the checker. A token like `0.516888...` cannot be parsed as a float, so DTChecker falls back to literal string comparison and any benign last-digit drift breaks the build. That is the opposite of what the checker is for, and it is why the nightly currently reports 8 doctest failures of which 7 are drift of ~6e-5 to ~0.8%. Remove them. Digits are left exactly as they were wherever the recorded value already carried enough precision, so no expected value is regenerated and a real regression cannot be silently blessed by this commit. Sixteen examples did need new values, because they had been recorded at only two or three decimals. Literal prefix matching tolerated that; a relative comparison cannot (`0.35` against `0.35952902` is 2.65% off, and `-0.00` against `-0.00881137` is 100% off). Those are rewritten at full precision, taken from an environment matching the docs job. grf.RegressionForest needed a change of a different kind. Its example generated data with `n_informative=2` out of 4 features, so two of the four printed importances were structurally ~0 (6.6e-5) while rtol allows them only +/-8e-7 -- and importances are a forest quantity, where near-tied splits can move them. Rounding does not rescue it: at round(3) the leading 0.88558872 sits 0.01% from the 0.8855 boundary. Instead make all four features informative, which lifts the smallest importance to 0.0365, about 500x further from zero. Note that the ellipsis form was never actually safer near zero, only brittle in a different direction: `0.00...` rejects a value of -1e-9 outright on a sign flip. The durable fix is to prefer DGPs whose printed values sit well away from zero. Verified by running the real docs-job command (sphinx-build -b doctest) under Python 3.12 with the current lkg.txt: 290 tests, 0 failures. Repeated under OPENBLAS_CORETYPE=Nehalem to shake out values sensitive to BLAS kernel selection (the failure mode behind the recent ForestDRIV flake): also 290 tests, 0 failures. This does not attempt the sklearn 1.9 refresh. That needs an lkg.txt bump and is left to a follow-up, which this commit makes tractable by reducing the nightly failures from 8 to the 1 that reflects a genuine behavior change. 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
This was referenced Aug 5, 2026
This branch has not been deployed
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.
Finishes the migration to scipy-doctest's
DTChecker(begun as part of #1017), so float output is compared numerically (rtol=0.01,atol=1e-8) instead of as text. That change only handled the immediately breaking instances of doctests using ellipses, but 365 numeric ellipses across 143 lines were left behind.Purpose
Ellipses prevent the checker's numeric comparisons from kicking in, since
0.516888...can't be parsed as a float, soDTCheckerfalls back to literal string comparison and last-digit drift can break the build. This is why the nightly currently reports 8 doctest failures, of which 7 are drift of ~6e-5 to ~0.8%, all of which are due to benign changes in sklearn 1.9. We've also already seen that GitHub actions can run with different kernels, which means that we can end up with slightly different values, particularly for sensitive forest methods (which necessitated some related changes in #1049 for our actual tests rather than doctests), so moving away from the more brittle approach with ellipses to the DTChecker numeric approach should protect us from more churn do to minor unimportant changes.What's in the diff
Mostly nothing but deleted
.... Digits are untouched wherever the recorded value already carried enough precision. That's the large, mechanical bulk of the diff.Sixteen examples needed real values, because they'd been recorded at only two or three decimals, which worked okay with the ellipses, but now fail under DTChecker's relative-precision comparison:
0.350.35952902-0.08-0.08460643-0.00-0.00881137Those are rewritten at full precision from an environment matching the docs job.
This raises an important thing to be aware of, which is that under relative tolerances, expected values near zero have much less wiggle room, so we may want to tailor our DGPs for doctests so that we generate expected values with somewhat larger absolute values to give us more of a buffer. The prior ellipsis form was never actually safer near zero, just brittle in the other direction, since
0.00...rejects-1e-9on a sign flip. The more stable fix is DGPs whose printed values sit well away from zero.grf.RegressionForestneeded a DGP change. Its example usedn_informative=2of 4 features, so two printed importances were nearly 0 (6.6e-5) whilertolallows them only ±8e-7, and importances are a forest quantity, where near-tied splits can move them even with the same version of sklearn using different kernels. Making all four features informative lifts the smallest importance to 0.0365, ~500× further from zero and suitable for the relative tolerance.Verification
Ran the real docs-job command (
sphinx-build ./doc/ ... -b doctest) on Python 3.12 with the currentlkg.txt, i.e. the same sklearn 1.8.0 the docs job installs:OPENBLAS_CORETYPE=Nehalem— 290 tests, 0 failuresThe second run is because BLAS kernel selection varies across the heterogeneous runner fleet and was the root cause of the recent
ForestDRIVflake (#1049), so any value sensitive to it would surface here.Residual risk
17 printed values remain in the
0 < |v| < 0.01range, tightest0.00007303(window ±7.3e-7). All are linear/analytic outputs, and all were stable across both BLAS kernels. In the future we may want to consider updating those DGPs, but that's a much larger diff and would obscure this one.