Repository navigation
Stop overflow warnings from rejected line-search steps - #434
Merged
Merged
Conversation
The line search tests trial steps that are not multiplied by the learning rate. With plain gradients on a target with large units, these steps overflow in Normal and give the warnings in issue #352. The line search already rejects a non-finite loss, so the trial losses are now computed inside np.errstate, and the fitted models do not change. fit now raises a ValueError that says the fit diverged when the gradient has non-finite values. Before, the base learner failed with "Input y contains NaN". The tuning page of the user guide now explains that plain gradients depend on the units of Y and that deep trees with very small leaves collapse the predicted scale. Closes #352. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…well-c505f3 # Conflicts: # RELEASE_NOTES.md
ryan-wolbeck
self-requested a review
November 7, 2026 23:15
ryan-wolbeck
approved these changes
Nov 7, 2026
alejandroschuler
added a commit
that referenced
this pull request
Nov 10, 2026
alejandroschuler
added a commit
that referenced
this pull request
Nov 10, 2026
pre-commit splits the test files over parallel pylint runs, one per CPU. duplicate-code only compares files in the same run, so its result depends on the CPU count and on which files exist. With the new test files of #433 and #434, the copy of the old-tree pickle helper in test_helpers.py and test_pickling.py lands in one run on a 4-CPU GitHub runner, and the hook fails. master already fails the same way when pylint gets all test files at once. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
alejandroschuler
added a commit
that referenced
this pull request
Nov 10, 2026
* Mark only the T case as xfail in the logscore distribution test test_dists_runs_on_examples_logscore passed a function object as the condition of its xfail marker. pytest treats a non-string condition as a truth value, and a function is always true, so every distribution and learner in the test was marked xfail. A real failure in any of them would have shown as xfailed, and the passing cases showed as xpassed with --slow. The xfail now sits on the T case alone, through pytest.param. The marker is strict and only accepts ValueError, because NGBRegressor(Dist=T) with default settings fails for both learners with "Input y contains NaN" (the natural gradient for log df runs away as df grows, see #275). The unused is_t_distribution helper is removed. Found while assessing #168. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Describe the T failure without quoting the error message #434 replaces 'Input y contains NaN' with its own 'The fit diverged' ValueError. The xfail reason now holds before and after #434 lands. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
alejandroschuler
added a commit
that referenced
this pull request
Nov 10, 2026
…mes (#433) * Add mu and sigma to LogNormal and raise AttributeError for unknown names LogNormal.scale was sigma, the standard deviation of log(Y), but params['scale'] is exp(mu), as in scipy.stats.lognorm, so users read the wrong number (#168). LogNormal now has the attributes mu and sigma. The attributes loc and scale still return mu and sigma, but they give a FutureWarning. params does not change, so code that reads params['s'] and params['scale'], such as the skpro adapters, keeps working. The distributions built on scipy.stats returned None for any name they did not have, so a typo such as dist.s failed silently. One __getattr__ in Distn now forwards names to self.dist and raises AttributeError for the rest. It reads dist from __dict__, so predicted distributions can now be pickled and deep-copied. Before, both raised RecursionError. Models saved with earlier versions load and predict the same, because a model stores the distribution class and the internal parameters, not distribution objects. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Assert the params keys in the LogNormal fit test pylint on tests flagged the two bare expressions as expression-not-assigned (W0106), which failed the lint job. The test now checks that params has the keys s and scale, and it imports the score classes at the top of the file. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Move the AttributeError release note away from the #434 notes #434 adds its bug-fix notes after the #409 line too, so the two PRs conflicted in RELEASE_NOTES.md. With this note after the #422 line, they merge cleanly in either order. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Disable pylint duplicate-code in the tests lint hook pre-commit splits the test files over parallel pylint runs, one per CPU. duplicate-code only compares files in the same run, so its result depends on the CPU count and on which files exist. With the new test files of #433 and #434, the copy of the old-tree pickle helper in test_helpers.py and test_pickling.py lands in one run on a 4-CPU GitHub runner, and the hook fails. master already fails the same way when pylint gets all test files at once. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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.
Closes #352.
Cause
The overflow warnings in
Normal(overflow encountered in expatself.scale = np.exp(params[1])andoverflow encountered in squareatself.var = self.scale**2) come from trial steps that the line search rejects.The line search tests
start - scale * residsfor scale 1, 2, ..., 512. These trial steps are not multiplied by the learning rate, so each one is 1/learning_ratetimes the step that the fit applies. If a trial step overflows, its loss is not finite, and the line search rejects it and halves the scale. The model does not change. Only the warnings get through.The most common way to get these warnings is plain gradients on a target with large units. With
natural_gradient=FalseandCRPScore, the gradient for log sigma is sigma * (2 phi(Z) - 1/sqrt(pi)), so its size grows with the units ofY. When the predicted standard deviation is above about 600, the first trial step (scale 1) puts log sigma above 355, andscale**2overflows. This reproduces the issue on master:On master this fit gives 94 overflow warnings, and all predictions are finite. Laplace and Exponential with
CRPScoreand Poisson withLogScoreoverflow in the same way with plain gradients.Deep trees with
min_samples_leaf=1cause a different problem. They fit each training point exactly, and the predicted scale collapses toward 0 on the training data. The collapse also makes trial steps overflow, and early stopping ends the fit before the collapse. This is probably why early stopping removed the warnings for one reporter. At a high learning rate (0.5 or more), the collapse can make a real parameter overflow. The gradient then has NaN values, and the base learner fails withInput y contains NaN, as in #61.Changes
NGBoost.line_searchcomputes the trial losses insidenp.errstate(over="ignore", divide="ignore", invalid="ignore"). The line search already rejects a non-finite loss, so the fitted model does not change. The loss at the current parameters stays outside theerrstate, so real overflows still warn.partial_fitchecks the gradient before it fits the base learners. If the gradient has non-finite values, it raises aValueErrorthat says the fit diverged at iterationkand gives settings to try. For scikit-learn learners this replacesInput y contains NaNat the same iteration, with the same exception type. A base learner that accepts NaN targets now gets this error and is not given a bad target.Y(standardizeY, or keepnatural_gradient=True). It also says that deep trees with very small leaves collapse the predicted scale, and that shallower trees, a lower learning rate or early stopping prevent this. The new cells are markdown only, between "Early Stopping" and "UsingsklearnModel Selection", so they do not touch the cells that Run the user guide notebooks in the docs build #431 changes.Evidence
The same fits with the master line search and with this branch:
Normal with natural LogScore, deep trees and
learning_rate=0.5on the diabetes data now stops withThe fit diverged at iteration 239: ...and not withInput y contains NaN.The guide's claim about plain gradients: with
Normal,CRPScore, sd(Y) about 1000 and 500 trees, the predicted mean has a standard deviation of 0.01 withnatural_gradient=Falseand 858 withnatural_gradient=True. The standard deviation of the true signal is 872.Tests
tests/test_numerics.py:test_rejected_line_search_steps_do_not_warnfits the reproducer above withRuntimeWarningset to raise an error. It fails on master.test_fit_raises_clear_error_when_gradient_is_not_finiteputs a NaN into theNormalgradient and checks for the new error.I ran the full test suite, black, isort, flake8 and pylint (10.00/10).
Not in this PR
Normal. Clipping also removes the warnings, but it changes fits in pathological runs. Today the overflowing trial steps are rejected. With a clip they are finite and can be accepted, and in one test withlearning_rate=1.0the test CRPS went from 63.5 to 5279.NGBRegressor(Dist=T)with default settings fails withInput y contains NaNat about iteration 25, at every scale ofYthat I tried. This is a separate bug in the T distribution. With this PR it gets the new "diverged" error.🤖 Generated with Claude Code