Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

enable ONNX export #165

Merged
merged 9 commits into from
Feb 7, 2025
Merged

Conversation

AlexanderPfefferle
Copy link
Contributor

modified the code to support ONNX export:

  • use custom implementation of torch.nanmean and torch.nansum everywhere
  • replaced usage of torch.Generator with code that isolates the rng in add_embeddings

Copy link
Collaborator

@LeoGrin LeoGrin left a comment

Choose a reason for hiding this comment

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

LGTM, thanks a lot @AlexanderPfefferle 🥳🥳 I also feel like it makes the code simpler, so that's great!
The pre-commit error is not related to this PR, I've just fixed it on main.
For the regressor test, IIRC, the change is because check_methods_subset_invariance now also fail close to the 1e-7 threshold for lack of precision, correct?

@LeoGrin
Copy link
Collaborator

LeoGrin commented Feb 5, 2025

Thinking about it, would it be easy to add a test that TabPFN is ONNX exportable, to make it sure it stays exportable?

@AlexanderPfefferle
Copy link
Contributor Author

Yes exactly, check_methods_subset_invariance is failing on the main branch right now with Max absolute difference among violations: 3.199093e-06.
I've added tests that check for exportability on CPU.

@LeoGrin
Copy link
Collaborator

LeoGrin commented Feb 6, 2025

Thanks a lot @AlexanderPfefferle ! onnx should be added to the dependencies (but not the main ones) to make the tests green. (I can also do it if you add me to your fork).

@LeoGrin
Copy link
Collaborator

LeoGrin commented Feb 7, 2025

I set the random state carefully to pass the regressor test for older versions, which is due to #175 and not to this pr.

@LeoGrin
Copy link
Collaborator

LeoGrin commented Feb 7, 2025

Merging, thanks a lot @AlexanderPfefferle !

@LeoGrin LeoGrin merged commit 634efcd into PriorLabs:main Feb 7, 2025
5 checks passed
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.

2 participants