Conversation
|
@copilot can you add unit test for the added code |
* Initial plan * Add unit tests for remove_inputs in InputObserver Co-authored-by: xadupre <22452781+xadupre@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: xadupre <22452781+xadupre@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR adds a remove_inputs API to InputObserver to allow dropping specific captured inputs before inferring dynamic shapes/arguments, and updates the ModelBuilder exporter path handling plus related docs/tests.
Changes:
- Add
remove_inputsplumbing acrossInputCandidate,InputObserverInfo, andInputObserver. - Adjust ModelBuilder export to handle empty
cache_dirand attempt to rename the produced ONNX file. - Add unit tests for removing kwarg inputs and a new sphinx-gallery example demonstrating ModelBuilder export.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| onnx_diagnostic/investigate/input_observer.py | Introduces remove_inputs methods to drop captured inputs and recompute internal flattened structures. |
| onnx_diagnostic/export/api.py | Tweaks ModelBuilder exporter cache/output handling and adds a load/save step intended to rename the output ONNX. |
| _unittests/ut_investigate/test_input_observer.py | Adds tests asserting remove_inputs updates inferred dynamic shapes and inferred arguments for kwargs. |
| _doc/examples/plot_export_with_modelbuilder.py | Adds a new sphinx-gallery example for exporting a HF model with ModelBuilder and using remove_inputs. |
| CHANGELOGS.rst | Adds changelog entries referencing the PR. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| cache_dir=os.path.dirname(filename) or ".", | ||
| **(exporter_kwargs or {}), | ||
| ) | ||
| save_model_builder(onx, os.path.dirname(filename)) | ||
| temp_filename = os.path.join(os.path.dirname(filename), "model.onnx") | ||
| # renaming | ||
| onx = onnx.load(temp_filename, load_external_data=True) | ||
| onnx.save(onx, filename, save_as_external_data=True) |
There was a problem hiding this comment.
In the modelbuilder exporter, save_model_builder(onx, os.path.dirname(filename)) passes an empty string when filename has no directory, which makes save_model_builder return an in-memory proto without writing a file. The subsequent onnx.load(temp_filename) will then fail. Use a normalized output dir (e.g., out_dir = os.path.dirname(filename) or ".") consistently for cache_dir, save_model_builder, and temp file paths.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
No description provided.