Handle constants without placeholder nodes in convert_constant_dim_order_pass #16841
Conversation
GregoryComer
requested review from
JacobSzwejbka and
larryliu0820
as code owners
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/16841
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit 07846a8 with merge base 61ae2f6 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
meta-cla
bot
added
the
CLA Signed
|
@GregoryComer has exported this pull request. If you are a Meta employee, you can view the originating Diff in D91334285. |
GregoryComer
added
the
release notes: none
…der_pass (pytorch#16841) Summary: After recent changes to convert_constant_dim_order_pass, there are a few lowering breakages when constant nodes don't have associated placeholders. I didn't realize this could happen, but apparently it does. So just skip placeholder meta updates in this case. Differential Revision: D91334285
GregoryComer
force-pushed
the
export-D91334285
branch
from
cebf3ad to
07846a8
Compare
Gasoonjia
left a comment
Gasoonjia
left a comment
There was a problem hiding this comment.
Thanks for update!
My major concern is whether or not that's the target behavior we want to have. In convert_constant_dim_order_pass we make const in contiguous first, then update the placeholder metadata. If in the _update_placeholder_meta we skip the update, will it introduce metadata-mismatch and lead to more issue?
| ) | ||
| if input_spec is None: | ||
| raise RuntimeError(f"Missing input spec for lifted tensor {target}.") | ||
| logger.warning(f"Missing input spec for constant {target}") |
There was a problem hiding this comment.
Do you know when the export program may not have input_spec? Doesn't every program have that?
There was a problem hiding this comment.
I can track down a better repro. This showed up with an internal CV model. My guess is that there is some pass that is modifying the constant and it hasn't been fully re-lifted at this point. I'll see if I can find an exact case. There are several broken internal tests for export currently.
2 participants
Summary: After recent changes to convert_constant_dim_order_pass, there are a few lowering breakages when constant nodes don't have associated placeholders. I didn't realize this could happen, but apparently it does. So just skip placeholder meta updates in this case.
Differential Revision: D91334285