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

Do not crash on torch.export #73

Merged
merged 3 commits into from
Dec 1, 2024
Merged

Conversation

gilfree
Copy link
Contributor

@gilfree gilfree commented Nov 26, 2024

Fix file name length issue when torch.export.export is called under depyf

The name used in the lazy_format_graph_code when using torch.export.export contains the "nn_module_stack" dictionary of the first module call of the model, which is an unreadable and long string.

As a prefix it contains a hard-coded string "pre insert_deferred_runtime_asserts" and then the result of f"exported program: {first_call_function_nn_module_stack(gm.graph)}", where the function call returns the dict.

This means that currently at least splitting by ':' will give useable name.

As export requires full graph we will have only one graph, so it will probably not be confusing.

Fix file name length issue when torch.export.export is called under depyf
@gilfree
Copy link
Contributor Author

gilfree commented Nov 26, 2024

This should handle #72

Signed-off-by: youkaichao <[email protected]>
Signed-off-by: youkaichao <[email protected]>
Copy link

codecov bot commented Dec 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.01%. Comparing base (ee7d231) to head (341dd8a).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master      #73      +/-   ##
==========================================
+ Coverage   90.88%   91.01%   +0.12%     
==========================================
  Files          10       10              
  Lines        1547     1547              
==========================================
+ Hits         1406     1408       +2     
+ Misses        141      139       -2     

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

@youkaichao youkaichao merged commit b9fa7a5 into thuml:master Dec 1, 2024
11 checks passed
@youkaichao
Copy link
Member

@gilfree thanks for the fix!

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.

[Bug]: using nn module stack for file name lead to too long files in some cases
2 participants