Skip to content

Test that fit_from_preprocessed output depends only on the batch - #1360

Merged
LeoGrin merged 1 commit into
mainfrom
leo/fit-from-preprocessed-contract-test
Oct 12, 2026
Merged

LeoGrin merged 1 commit into
mainfrom
leo/fit-from-preprocessed-contract-test

Conversation

@LeoGrin

@LeoGrin LeoGrin commented Oct 3, 2026

Copy link
Copy Markdown
Collaborator

Generated by Claude Code

Adds one test, for both estimators, that pins the contract of a custom fine-tuning loop. The loop calls fit_from_preprocessed on a batch and then forward. The test requires that the output does not depend on whether the estimator has already run fit() or an earlier batch on other data. Two regressions of this kind have already slipped through review: #1270 (a stale correction from fit()) and #1198 (a target frame that only fit() sets).

What the test does
  • A fresh estimator runs one loop step on a batch. A failure here means the loop now needs state it does not set, as on Give each regression member its own target pipeline [RES-2639] #1198.
  • A second estimator first runs fit() on other data, or a loop step on another batch, then runs the same step. Its output must equal the fresh one (per-estimator logits, and borders for the regressor).
  • The other data differs in seed, width (6 vs 4 features) and target scale (x1000). The estimators use majority_downsample, so fit() also stores a prior-shift correction.
  • _forward_on_batch is the whole loop step. If a change needs more steps there, every user loop outside this repo breaks, and the change to the helper makes that visible in review.
Checked against both known regressions
  • main (15f5e6b): 4 passed, 2 s on CPU.
  • Give each regression member its own target pipeline [RES-2639] #1198 head (e541339): both regressor cases fail with AttributeError: 'TabPFNRegressor' object has no attribute 'y_train_mean_'.
  • main without the two Correct the target prior shift of majority_downsample row subsampling #1270 resets in fit_from_preprocessed: the classifier fit case fails, with all 50 outputs different by up to 0.26.
    • The regressor case still passes, because the regressor applies its correction in predict, not forward. A stale correction there never reaches the fine-tuning loss.
  • test__classifier__fit_from_preprocessed_clears_stale_correction and its regressor twin in tests/test_downsample_correction.py overlap this test. I left them in place.
  • Tests only, so this needs the "no changelog needed" label.

🤖 Generated with Claude Code

A custom fine-tuning loop calls fit_from_preprocessed and then forward. Both
regressions we hit there were estimator state from outside the batch: a
correction left over from fit(), and a target frame only fit() sets.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@LeoGrin
LeoGrin marked this pull request as ready for review October 3, 2026 15:29
@LeoGrin LeoGrin added the no changelog needed PR does not require a changelog entry label Oct 4, 2026
@LeoGrin
LeoGrin requested a review from bejaeger October 4, 2026 00:17
@LeoGrin
LeoGrin added this pull request to the merge queue Oct 12, 2026
Merged via the queue into main with commit 613021b Oct 12, 2026
22 of 23 checks passed
@LeoGrin
LeoGrin deleted the leo/fit-from-preprocessed-contract-test branch October 12, 2026 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no changelog needed PR does not require a changelog entry

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants