Repository navigation
Fix the T distribution crash: cap df and use the exact Fisher information - #443
Open
alejandroschuler wants to merge 2 commits into
Open
alejandroschuler wants to merge 2 commits into
alejandroschuler wants to merge 2 commits into
Conversation
NGBRegressor(Dist=T) failed with "Input y contains NaN" after a few dozen iterations on every seed. The information for log df falls like 1 / df^2, so the natural gradient for log df grows in proportion to df. Rows whose df drifted up got ever larger steps, the loss is nearly flat toward large df, and log df ran past 709 until exp overflowed (issue #275). - Cap df at T.max_df = 1000. A row at the cap gets no push further up, and its log df is not coupled to log scale in the metric, so df can still come back down. - Give TLogScore the exact Fisher information (Lange, Little and Taylor 1989) in place of the 100-draw Monte Carlo estimate. Fits are about three times faster and reproducible with random_state. - Keep the start at df = 3 (now T.init_df) and say why: a df fitted to the marginal Y starts at the cap and gives a worse fit. - Fix the docstrings of the T classes, which called loc and scale the mean and standard deviation. - Remove the xfail for T in the logscore distribution test, and add tests for the fit, the cap and the metric. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Collaborator
|
@alejandroschuler I'll try to get a release PR created in the next couple days. There are few breaking changes merged that will need to be documented as part of the release (to 0.5.0) then move to merge the rest you have created. Good with that? |
Collaborator
Author
|
yes, thanks @ryan-wolbeck! I admin-merged a couple of things that seemed unobjectionable, hope that's ok! |
Collaborator
No problem at all! Just want to make the releases small so if we need to roll things back it's easier to parse what happened and since there are things like sign flips now merged users will need to update code that may be running in prod that they install from pypi. |
ryan-wolbeck
self-requested a review
November 7, 2026 14:56
…talcini-9053fd Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> # Conflicts: # RELEASE_NOTES.md
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.
Fixes the crash reported in #275, which was closed in 2021 without a fix.
Problem
NGBRegressor(Dist=T)with default settings fails withValueError: Input y contains NaNafter a few dozen iterations:It failed in 15 of 15 fits that I tried (5 seeds, Y in units of 1, 100 and 1e4), at iterations 26 to 60. Scaling Y does not help. It is not a regression: v0.4.0 and v0.5.2 fail the same way.
tests/test_distns.pymarked this case as a known failure.Cause
The gradients in
TLogScore.d_scoreare correct. The problem is how log df behaves as df grows.np.exp(params[2])overflows. The gradient is then NaN, and the tree fit fails.The 100-draw Monte Carlo metric that
Tused made this worse. For each row it underestimated the information for log df, by a factor of 1.4 to 2.8 at the median.Changes
T.max_df = 1000.T.__init__clips log df beforeexp. A t with df 1000 is very close to a normal. A row at the cap gets no push further up, and its log df is not coupled to log scale in the metric, so the fit can still bring df down. Without this, the raw parameter of capped rows kept rising (to about 1400 in 500 trees) and they stayed stuck at the cap. On Cauchy noise that cost 0.43 nats of test NLL (table below).TLogScore.metricis the Fisher information of (loc, log scale, log df) from Lange, Little and Taylor (1989). It agrees with quadrature ofd_scoreto 1e-6 or better. Fits are about three times faster (2.7 s against 8.2 s for 500 trees on 500 rows). Fits with the samerandom_stateare now identical. Before, the metric drew its samples from the global numpy random state.T.fitstill starts every row at df = 3, now throughT.init_dfwith a comment. I tried fitting df to Y withscipy.stats.t.fit, and it is worse. Y mixes the signal with the noise, so its tails say little about the noise. On these datascipyreturned df = 1.6e10, every row started at the cap, and the fitted df stayed far above the truth (median 182 against 5 on t₅ noise). It also gavescipyRuntimeWarnings insidet.fit. See "fitted start" in the second table.T,TFixedDfandTFixedDfFixedVarcalled loc and scale the mean and standard deviation. They now say that loc is the mean (when df > 1) and give the standard deviation.Results
Data:
tests/test_distns.py.Every model uses the library defaults, because the bug is in the default fit. There are 10 seeds for each cell. The finish counts pool Y in units of 1 and 1e4 for the synthetic data. Test NLL is the mean over seeds at units of 1. The numbers in brackets are the mean paired difference from this PR, ± its Monte Carlo SE (positive means worse than this PR).
TFixedDfNormalEvery fit now finishes. This PR beats
Normalon all four data sets. It beatsTFixedDfon Cauchy noise. It is a little worse thanTFixedDfon t₅ and normal noise, and worse on California (see "Limits" below).The design choices, with the same data and seeds:
scipy.stats.t.fit)Limits
Tstill learns more slowly thanTFixedDf. On seed 0 of the t₅, Cauchy and California data, the median line-search scale is 0.016 to 0.5 forTand 1.0 forTFixedDf, and 27% to 87% of theTiterations take a scale below 0.5. The cause is the same scaling as above. The line search uses one scale for all parameters. A candidate step that is large in log df on one row makes the loss worse, so the step shrinks for loc and scale too. In 500 trees df moves only a little from its start at 3 (median 3.5 on normal noise). A fix is to put the tail on a scale with bounded information near the normal limit, such as 1/df, for which the information tends to 1.5. That changes the meaning of the third parameter, so I left it for a separate PR.Trial steps that the line search rejects can still give a few scipy
RuntimeWarnings (divide by zero encountered in log), when a trial step sends df close to 0. The fit does not change. #434 silences warnings from rejected trial steps.Interaction with other open PRs
I ran trial merges of this branch with each of them.
Tcase oftest_dists_runs_on_examples_logscoreas a strict xfail. This PR makes that case pass, so a strict xfail fails. If Mark only the T case as xfail in the logscore distribution test #437 merges first, theTmarker has to go when this PR is merged. If this PR merges first, Mark only the T case as xfail in the logscore distribution test #437 should drop itsTmarker (its fix of the always-true condition still applies).self.vartoself._var. It conflicts with this PR on adjacent lines ofT.__init__. Keep both changes. The newmetric()readsself.scale**2, so it does not depend on the name.dftoparamsof the T classes. This PR leavesparamsalone so that the two do not overlap. The conflicts are the import lines oftests/test_distns.pyand the release notes. Keep both sides.Tests
test_t_df_stays_finitefits the reproducer above with 100 trees, at Y units 1 and 1e4. It fails on master withInput y contains NaN.test_t_df_is_capped: a log df of 800 gives df = 1000, a finite score and a finite natural gradient.test_t_no_push_above_cap: at the cap, a row that favors a larger df gets a log-df step of 0, and an outlier still gets a step down.test_t_metricchecks the metric against quadrature of E[s sᵀ] at df 1.5, 3, 30 and 500.Tmoves from the gradient-only list toTEST_METRICintests/test_score.py.xfailontest_dists_runs_on_examples_logscoreis gone.Tpasses for both learners.--slow, the logscore distribution and metric tests pass (39 passed). The full suite without--slowpasses (143 passed, 59 skipped).black,isort,flake8andpylintare clean on the changed files, with the options from.pre-commit-config.yaml.🤖 Generated with Claude Code