Skip to content

fix(inference): let the batched engine accept only_return_standard_out - #1353

Open
aniruddhaadak80 wants to merge 3 commits into
PriorLabs:mainfrom
aniruddhaadak80:fix/batched-iter-outputs-kwarg
Open

aniruddhaadak80 wants to merge 3 commits into
PriorLabs:mainfrom
aniruddhaadak80:fix/batched-iter-outputs-kwarg

Conversation

@aniruddhaadak80

Copy link
Copy Markdown
Contributor

Issue

Fixes #1348

Motivation and Context

get_embeddings (�ase.py ~736) calls

python for output, config in model.executor_.iter_outputs( X, autocast=model.use_autocast_, task_type=task_type, only_return_standard_out=False, )

Three of the four engines accept that keyword. InferenceEngineBatchedNoPreprocessing.iter_outputs (~775) declares only X, �utocast and ask_type, and hardcodes only_return_standard_out=True on the model call, so calling it this way raises:

TypeError: iter_outputs() got an unexpected keyword argument 'only_return_standard_out'

The abstract declaration InferenceEngine.iter_outputs (~361) omits the parameter as well, which is how the gap survived in one implementation while the other three grew it.

The batched engine is what finetuning leaves its estimator in ( it_mode="batched"), so inetuned_estimator_.get_embeddings(X_test) crashes for exactly the estimators the embeddings feature is most useful on.

The fix declares only_return_standard_out: bool = True on both the ABC and the batched engine, and threads it to the model call in place of the hardcoded True, matching the convention the other three engines already use.

Public API Changes

  • No Public API changes
  • Yes, Public API changes (Details below)

InferenceEngine.iter_outputs gains only_return_standard_out: bool = True on its abstract signature, and InferenceEngineBatchedNoPreprocessing.iter_outputs now accepts it. Existing callers are unaffected - the parameter defaults to True, which is the value the batched engine previously hardcoded.

How Has This Been Tested?

Two tests in ests/test_inference.py:

  • est__iter_outputs__batched_honours_only_return_standard_out - drives the batched engine and asserts the value reaches the model call.
  • est__iter_outputs__every_engine_accepts_only_return_standard_out - parametrized over all four engines, so any future engine that forgets the parameter fails this test. This is the check that would have caught the original gap.

All five cases fail on the unfixed source and pass with the fix:

`

unfixed source

FAILED ...::test__iter_outputs__batched_honours_only_return_standard_out
FAILED ...::test__iter_outputs__every_engine_accepts_only_return_standard_out[InferenceEngineOnDemand]
FAILED ...::test__iter_outputs__every_engine_accepts_only_return_standard_out[InferenceEngineCachePreprocessing]
FAILED ...::test__iter_outputs__every_engine_accepts_only_return_standard_out[InferenceEngineBatchedNoPreprocessing]
FAILED ...::test__iter_outputs__every_engine_accepts_only_return_standard_out[InferenceEngineExplicitKVCache]
5 failed, 31 deselected

with the fix

5 passed, 31 deselected in 2.87s
`

est__kv_cache__embeddings_test_only_and_chunk_safe[cpu] fails in this file both with and without the change (it needs model weights), so it is unrelated. 

uff check and
uff format --check are clean.

Checklist

  • The changes have been tested locally.
  • Documentation has been updated (if the public API or usage changes).
  • A changelog entry has been added (see changelog/README.md), or "no changelog needed" label requested.
  • The code follows the project's style guidelines.
  • I have considered the impact of these changes on the public API.

get_embeddings() calls iter_outputs() with only_return_standard_out=False.
Three of the four engines accepted the keyword; InferenceEngineBatched
NoPreprocessing did not and hardcoded True on the model call, so get_embeddings
raised TypeError whenever the batched engine was active - which is the mode
finetuning leaves its estimator in.
Copilot AI balanced review requested due to automatic review settings October 2, 2026 06:27

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit a37935a. Configure here.

Comment thread changelog/1353.fixed.md Outdated
Comment thread src/tabpfn/inference.py Outdated
@aniruddhaadak80

Copy link
Copy Markdown
Contributor Author

Thanks - both points taken. Fixed in db77a51.

  • changelog/1353.fixed.md is rewritten in the user-facing style and no longer names the engine class or the exception.
  • The only_return_standard_out docstring now describes just the argument; the sentence pointing at get_embeddings() as the reason it exists is removed.

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.

get_embeddings raises TypeError when the batched inference engine is active

2 participants