[export] fix re-export custom metadata #135282
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/135282
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit f86c445 with merge base f65a564 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
pytorch-bot
bot
added
ciflow/inductor
release notes: quantization
yiming0416
force-pushed
the
yiming0416/fix_re_export_custom_meta
branch
2 times, most recently
from
99de798 to
78d5349
Compare
yiming0416
requested review from
angelayi,
avikchaudhuri,
jerryzh168,
tugsbayasgalan,
ydwu4 and
zhxchen17
as code owners
|
@yiming0416 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
torch/_export/utils.py
Outdated
| if k == "custom": | ||
| continue |
There was a problem hiding this comment.
@yiming0416 just curious why do we need to skip over this?
There was a problem hiding this comment.
Because the issue of @jerryzh168 is that the custom field is copied to get_attr nodes from the call_function nodes unexpectedly which causes the quantization test to fail.
I don't understand why we need the copy during the populating process though...
There was a problem hiding this comment.
as discussed offline with @yiming0416 I think the fix should be in: https://github.com/pytorch/pytorch/blob/main/torch/_export/utils.py#L136-L143
There was a problem hiding this comment.
Hmm this seems bit hacky, let's discuss offline whether we actually need this logic in the first place.
yiming0416
force-pushed
the
yiming0416/fix_re_export_custom_meta
branch
from
78d5349 to
0d894d5
Compare
|
@yiming0416 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
yiming0416
force-pushed
the
yiming0416/fix_re_export_custom_meta
branch
3 times, most recently
from
8a783dc to
7b47c69
Compare
|
@yiming0416 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
yiming0416
force-pushed
the
yiming0416/fix_re_export_custom_meta
branch
from
7b47c69 to
b12e134
Compare
|
@yiming0416 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
yiming0416
force-pushed
the
yiming0416/fix_re_export_custom_meta
branch
from
b12e134 to
f86c445
Compare
|
@yiming0416 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@pytorchbot merge -f 'Landed internally' (Initiating merge automatically since Phabricator Diff has merged, using force because this PR might not pass merge_rules.json but landed internally) |
Merge startedYour change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes). Please use Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
Fixes pytorch#134778 When a model is exported and debug handles are added to the "custom" field of non-placeholder and non-output nodes in the graph, re-exporting it will change the metadata of placeholder nodes (the "custom" field will be added or copied to these nodes, depending whether `ExportedProgram` or `ExportedProgram.module()` is passed to `generate_numeric_debug_handle()`). This occurs because when we re-export the model, `placeholder` nodes are unlifted to `get_attr` nodes. These nodes remain as `get_attr` after being exported to `gm_torch_level`. Their metadata are modified [here](https://github.com/pytorch/pytorch/blob/main/torch/export/_trace.py#L1347) based on `params_buffers_to_node_meta` which is collected [here](https://github.com/pytorch/pytorch/blob/main/torch/export/_trace.py#L1312). Pull Request resolved: pytorch#135282 Approved by: https://github.com/jerryzh168, https://github.com/zhxchen17, https://github.com/tugsbayasgalan
6 participants
Fixes #134778
When a model is exported and debug handles are added to the "custom" field of non-placeholder and non-output nodes in the graph, re-exporting it will change the metadata of placeholder nodes (the "custom" field will be added or copied to these nodes, depending whether
ExportedProgramorExportedProgram.module()is passed togenerate_numeric_debug_handle()).This occurs because when we re-export the model,
placeholdernodes are unlifted toget_attrnodes. These nodes remain asget_attrafter being exported togm_torch_level. Their metadata are modified here based onparams_buffers_to_node_metawhich is collected here.