add progress bar#899
Conversation
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces an optional progress bar for inference in both TabPFNClassifier and TabPFNRegressor using the tqdm library. The show_progress_bar parameter was added to the constructors, and the main inference loops were updated to display progress when enabled. Corresponding tests were also added to verify the new configuration. Feedback indicates that the progress bar is missing from the forward method in TabPFNRegressor, which should be addressed to ensure consistency with the classifier and provide feedback during fine-tuning tasks.
oscarkey
left a comment
oscarkey
left a comment
There was a problem hiding this comment.
Hey! Thanks so much for the contribution. This looks pretty good to me, I just have a few suggestions.
| "fit_with_cache", | ||
| "batched", | ||
| ] = "fit_preprocessors", | ||
| show_progress_bar: bool = False, |
There was a problem hiding this comment.
Would you add this to the end of the parameter list? Just to avoid *args accidents, and to match the doc string.
| total=self.n_estimators, | ||
| desc="TabPFN inference", | ||
| unit="estimator", | ||
| disable=not getattr(self, "show_progress_bar", False), |
There was a problem hiding this comment.
| disable=not getattr(self, "show_progress_bar", False), | |
| disable=self.show_progress_bar |
is there ever a case where this attribute isn't set?
There was a problem hiding this comment.
it was mostly for compatibility with past pickled models, but i guess it's unnecessary.
|
this should probably have addressed all comments |
oscarkey
left a comment
oscarkey
left a comment
There was a problem hiding this comment.
lgtm! Thank you very much.
I think you'll need to rename your changelog to 899.added.md to get the check to pass. And then I would recommend running the pre-commit hooks for the ruff failure.
|
hopefully it goes now |
3 participants
Issue
#898
This allows us to discuss the proposal and helps avoid unnecessary work.
Motivation and Context
A progress bar would be very useful when running with large dbs or many n_estimators.
Public API Changes
Changes the public API but with changes that are backward and forward compatible.
How Has This Been Tested?
Ran a regressor locally.
Checklist
changelog/README.md), or "no changelog needed" label requested.Fixes PRI-282