Skip to content

Return df in T.params and fix the Cauchy starting fit - #450

Open
alejandroschuler wants to merge 2 commits into
masterfrom
claude/reverent-euclid-39027f
Open

alejandroschuler wants to merge 2 commits into
masterfrom
claude/reverent-euclid-39027f

Conversation

@alejandroschuler

@alejandroschuler alejandroschuler commented Oct 27, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Two small fixes in the Student t family (ngboost/distns/t.py).

T.params now includes df. T fits the degrees of freedom, but params returned only loc and scale. Now it returns loc, scale and df, so scipy.stats.t(**dist.params) gives the predicted distribution. Before, that call failed because df was missing. The fixed-df classes (TFixedDf, TFixedDfFixedVar, Cauchy, CauchyFixedVar) keep loc and scale, because their df is a constant and the Cauchy keys match scipy.stats.cauchy.

Users of T.params that I checked:

  • In this repo, only tests/test_basic.py reads pred_dist(...).params. It loops over the keys, so the new key is safe there.
  • skpro's NGBoost adapter (skpro/regression/adapters/ngboost/_ngboost_proba.py) reads params["loc"] and params["scale"] by key, so it does not break. It builds its TDistribution without df, so it gets skpro's default df=1. With df in T.params, skpro can pass the fitted df through.
  • Code that puts T.params in a DataFrame, or that passes it to a constructor with only loc and scale, gets one more key. The release notes say so.

The starting fit of Cauchy and CauchyFixedVar used df = 3. Cauchy subclasses TFixedDf with fixed_df = 1.0, but TFixedDf.fit read TFixedDf.fixed_df, so the marginal fit that sets init_params used df = 3. On Cauchy data with scale 0.5, Cauchy.fit gave a scale of 0.89. CauchyFixedVar had the same problem through TFixedDfFixedVar.fit. User subclasses with their own fixed_df had it too (some on GitHub subclass TFixedDf with fixed_df = 5).

TFixedDf.fit and TFixedDfFixedVar.fit are now classmethods that read cls.fixed_df. NGBoost calls self.Manifold.fit(Y) on a class that subclasses the distribution, and SurvivalDistnClass calls Dist.fit(Y["Time"]). Both work with a classmethod. I also checked that an NGBRegressor with Cauchy starts at the Cauchy MLE and still pickles.

TFixedDfFixedVar.fit now also passes fscale=1.0. The model holds the scale at 1, but the starting fit estimated loc with a free scale, so the starting loc was not the MLE under the model.

T.fit is not changed here. #443 rewrites it.

Tests

New file tests/test_t.py. All of its tests fail on master except the TFixedDf control case:

  • test_t_params_include_df: T.params has df, and scipy.stats.t(**params) gives the same log density as the ngboost distribution.
  • test_fixed_df_fit_uses_class_df (TFixedDf, Cauchy, a subclass with fixed_df = 5): fit equals scipy.stats.t.fit with that df.
  • test_cauchy_fit_is_cauchy_mle: Cauchy.fit on Cauchy data equals scipy.stats.cauchy.fit.
  • test_fixed_df_fixed_var_fit_uses_class_df_and_unit_scale (TFixedDfFixedVar, CauchyFixedVar, a subclass with fixed_df = 5): fit equals scipy.stats.t.fit with that df and scale 1.

The tests are in a new file because #442, #443 and #446 all append to tests/test_distns.py. git merge-tree merges this branch with each open PR (#442, #443, #445 to #449) with no conflict.

pytest tests passes. black, isort, flake8 and pylint (with the flags in .pre-commit-config.yaml) pass.

The second commit fixes a lint failure that the new test file exposed. tests/test_helpers.py had its own copy of the two old-pickle helpers in tests/test_pickling.py. pylint reports that as duplicate-code (R0801) only when pre-commit puts both files in the same batch. A new file in tests/ changes the batches, and here it put them together. test_helpers.py now imports the helpers from test_pickling.py, so pylint tests passes on all test files at once. I did not change test_pickling.py, because #445 and #446 edit it.

Docs

This PR does not change docs/content/1-useage.ipynb. The distribution tables and their notes come from #447, which is not on master yet. After both PRs merge, #447's text needs two edits:

  1. In the regression table, change the Parameters cell of the T row from loc, scale to loc, scale, df.
  2. In the notes cell under the table, cut the sentences "T also fits the degrees of freedom. They are not in params, but the df attribute of a predicted distribution gives them."

🤖 Generated with Claude Code

alejandroschuler and others added 2 commits October 27, 2026 14:29
T.params now returns df, the fitted degrees of freedom, next to loc and
scale. So scipy.stats.t(**dist.params) gives the predicted distribution.

TFixedDf.fit and TFixedDfFixedVar.fit read fixed_df from the class they
are called on. Before, they read it from TFixedDf and TFixedDfFixedVar, so
Cauchy and CauchyFixedVar (and user subclasses with their own fixed_df)
started from the df = 3 fit. TFixedDfFixedVar.fit also holds the scale at
1 now, as the model does.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
tests/test_helpers.py had its own copy of the two old-pickle helpers in
tests/test_pickling.py. pylint reports this as duplicate-code (R0801)
when pre-commit puts both files in the same batch, and the new test file
changed the batches so that it did. test_helpers.py now imports the
helpers from test_pickling.py.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
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.

1 participant