Skip to content

Remove automatic ONNX export from training#520

Merged
drewoldag merged 2 commits intomainfrom
copilot/remove-export-to-onnx
Oct 31, 2025
Merged

Remove automatic ONNX export from training#520
drewoldag merged 2 commits intomainfrom
copilot/remove-export-to-onnx

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Oct 30, 2025

Training automatically exported models to ONNX format regardless of whether they would go to production, wasting compute time.

Changes

  • Removed export_to_onnx() call and supporting code from Train.run() in src/hyrax/verbs/train.py
  • Removed import of export_to_onnx from train verb
  • The function remains available in model_exporters.py for explicit manual use

Impact

Training completes faster. Models are only exported to ONNX when users explicitly call export_to_onnx() instead of automatically after every training run.

Original prompt

This section details on the original issue you should resolve

<issue_title>Remove export_to_onnx from train</issue_title>
<issue_description>In the interest of avoiding surprises, let's remove the automatic ONNX-ification of models immediately after training.

Often the trained model is not the one that will go to production, and thus we're wasting compute time ONNX-ifying them prepmaturely. </issue_description>

Comments on the Issue (you are @copilot in this section)


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Co-authored-by: drewoldag <47493171+drewoldag@users.noreply.github.com>
Copilot AI changed the title [WIP] Remove export_to_onnx from training process Remove automatic ONNX export from training Oct 30, 2025
Copilot AI requested a review from drewoldag October 30, 2025 22:58
@codecov
Copy link
Copy Markdown

codecov bot commented Oct 30, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 53.55%. Comparing base (25b020c) to head (d334335).
⚠️ Report is 63 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #520      +/-   ##
==========================================
- Coverage   54.05%   53.55%   -0.51%     
==========================================
  Files          48       48              
  Lines        4767     4760       -7     
==========================================
- Hits         2577     2549      -28     
- Misses       2190     2211      +21     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Collaborator

@drewoldag drewoldag left a comment

Choose a reason for hiding this comment

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

This looks correct to me. The goal of the PR is to stop attempting to automatically produce an ONNX model at the end of train. This does just that.

@drewoldag drewoldag marked this pull request as ready for review October 31, 2025 16:55
@drewoldag drewoldag requested review from a team, aritraghsh09, Copilot, gitosaurus and maxwest-uw and removed request for a team and maxwest-uw October 31, 2025 16:55
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request removes the automatic ONNX model export functionality from the training verb. After training completes, the model will no longer be automatically exported to ONNX format.

  • Removed ONNX export import statement
  • Removed post-training ONNX export code block
Comments suppressed due to low confidence (1)

src/hyrax/verbs/train.py:1

  • The removal of ONNX export functionality from the training verb leaves the [onnx] configuration section in hyrax_default_config.toml (lines 160-164) unused. This section includes the opset_version parameter that is no longer referenced anywhere in the codebase. Consider either removing this configuration section or documenting that ONNX export has been moved/deprecated.
import logging

Copy link
Copy Markdown
Contributor

@gitosaurus gitosaurus left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Collaborator

@aritraghsh09 aritraghsh09 left a comment

Choose a reason for hiding this comment

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

🚢 it

@drewoldag drewoldag merged commit 180ecc3 into main Oct 31, 2025
21 of 23 checks passed
@drewoldag drewoldag deleted the copilot/remove-export-to-onnx branch October 31, 2025 18:21
@drewoldag drewoldag linked an issue Nov 3, 2025 that may be closed by this pull request
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.

Remove export_to_onnx from train Error when converting to AppleCider pytorch model to ONNX

5 participants