Reorder the Fx execution order to in-time get_attr rather than putting all get_attr ahead #95014
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/95014
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit 67f69df: This comment was automatically generated by Dr. CI and updates every 15 minutes. |
|
|
|
This pull request was exported from Phabricator. Differential Revision: D43376080 |
|
/easycla |
|
This pull request was exported from Phabricator. Differential Revision: D43376080 |
a183ceb to
897800a
Compare
…g all get_attr ahead (pytorch#95014) Summary: Pull Request resolved: pytorch#95014 Basically today we: [getattr....getattr, call partition1, call parition2] this makes getattr just in time: so [getattr, call partition1, getattr, call partition 2 ..] Test Plan: CMF and MAI test result: https://fb.quip.com/K5J9A7G246Ox Reviewed By: angelayi Differential Revision: D43376080 fbshipit-source-id: 69c2fcfdf469706f9649f6d1d17d695f4693d04a
|
This pull request was exported from Phabricator. Differential Revision: D43376080 |
897800a to
67f69df
Compare
|
@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). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
| else: | ||
| # Go through the graph to construct the mapping dict | ||
| for node in m.graph.nodes: | ||
| if node.op == "placeholder" or "get_attr": |
There was a problem hiding this comment.
This will always return true because the second part of the or condition is just a non-empty string:
>>> foo = "get_attr"
>>> foo == "placeholder" or "get_attr"
'get_attr'
>>> foo = "placeholder"
>>> foo == "placeholder" or "get_attr"
True
You want to use another == or use in
if node.op in ("placeholder", "get_attr"):
There was a problem hiding this comment.
thanks for pointing out!
…g all get_attr ahead (#95014) Summary: Basically today we: [getattr....getattr, call partition1, call parition2] this makes getattr just in time: so [getattr, call partition1, getattr, call partition 2 ..] Test Plan: CMF and MAI test result: https://fb.quip.com/K5J9A7G246Ox Differential Revision: D43376080 Pull Request resolved: pytorch/pytorch#95014 Approved by: https://github.com/angelayi
…n putting all get_attr ahead (pytorch#95014)" This reverts commit 0d2e915.
…g all get_attr ahead (pytorch#95014) Summary: Basically today we: [getattr....getattr, call partition1, call parition2] this makes getattr just in time: so [getattr, call partition1, getattr, call partition 2 ..] Test Plan: CMF and MAI test result: https://fb.quip.com/K5J9A7G246Ox Differential Revision: D43376080 Pull Request resolved: pytorch#95014 Approved by: https://github.com/angelayi
Summary:
Basically today we:
[getattr....getattr, call partition1, call parition2]
this makes getattr just in time:
so [getattr, call partition1, getattr, call partition 2 ..]
Test Plan:
CMF and MAI test result:
https://fb.quip.com/K5J9A7G246Ox
Differential Revision: D43376080