[export] Deserialize args with python keyword names #136036
Conversation
angelayi
requested review from
avikchaudhuri,
tugsbayasgalan,
ydwu4 and
zhxchen17
as code owners
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/136036
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit 35280a8 with merge base 66db61f ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
angelayi
force-pushed
the
angelayi/deserialize_keyword
branch
from
4f4a621 to
d018277
Compare
|
@angelayi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
angelayi
added
bug
release notes: export
topic: bug fixes
angelayi
changed the title
angelayi
force-pushed
the
angelayi/deserialize_keyword
branch
from
d018277 to
35280a8
Compare
|
@angelayi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
I mean I can accept this as a short term measure, but why this schema has a keyable argument with keyword name in this first place... Does that mean no one will ever be able to use this argument with the name in Python? |
|
@zhxchen17 I think it's probably a bug that the op's schema has a kwarg with a keyword name, or it was not expected to be used this way, since it seems like the op was added into native_functions.yaml long long ago |
|
@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 |
Currently when we deserialize inputs to nodes, we deserialize arguments with default values as kwargs. So deserializing `aten.uniform`, which has the signature `uniform(Tensor(a!) self, float from=0, float to=1, *, Generator? generator=None) -> Tensor(a!)`, will get become `uniform(x, from=0, to=1)`. However, this fails when running in python because `from` is a python keyword. So the solution here is to not deserialize it as a kwarg. Pull Request resolved: pytorch#136036 Approved by: https://github.com/zhxchen17
4 participants
Currently when we deserialize inputs to nodes, we deserialize arguments with default values as kwargs. So deserializing
aten.uniform, which has the signatureuniform(Tensor(a!) self, float from=0, float to=1, *, Generator? generator=None) -> Tensor(a!), will get becomeuniform(x, from=0, to=1). However, this fails when running in python becausefromis a python keyword. So the solution here is to not deserialize it as a kwarg.