-
Notifications
You must be signed in to change notification settings - Fork 19
#343: Support collections of tensors in args/kwargs for compile #701
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.10.
| Benchmark suite | Current: 87ecff6 | Previous: 48b5d7d | Ratio |
|---|---|---|---|
tests/performance/test_perf.py::test_perf_regression[sdpa-float16] |
2983.0722283127584 iter/sec (stddev: 0.0002213579824486067) |
3302.897668253486 iter/sec (stddev: 0.00007090329654256612) |
1.11 |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this 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 PR adds support for collections of tensors (nested dictionaries and lists) in the args and kwargs parameters of the compile function. Previously, only individual InputInfo or DimensionInputInfo objects were supported as runtime inputs, but now these can be organized within container structures.
Key Changes:
- Recursive processing of nested data structures (dicts, lists, tuples) containing
InputInfoobjects during compilation - Flattening of nested input structures at runtime to match the compilation structure
- New validation for input extraction ensuring all compiled inputs are provided at runtime
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tripy/nvtripy/backend/api/compile.py | Added recursive process_arg_and_flag function to handle nested containers during compilation and maintain flattened trace inputs |
| tripy/nvtripy/backend/api/executable.py | Added extract_inputs function to recursively flatten runtime tensor containers and validate against expected compiled inputs |
| tripy/tests/backend/api/test_compile.py | Added three comprehensive test cases covering nested dicts, nested sequences, and mixed container scenarios |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
I see that some additional docs tests are failing due to the new required argument (which caches the input structure) that I added for creating an executable. Initially I implemented it as an optional argument and then re-created it in the executable if it wasn't provided, but this didn't seem to make much sense to me and I was hoping to have the relevant logic only in one place. I could do this entirely in the |
4831cf1 to
0af2359
Compare
0af2359 to
db60c25
Compare
No description provided.