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.

Sign up for GitHub

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

Jump to bottom

Use original arg names if possible #95898

Closed
wants to merge 4 commits into from
Closed

Use original arg names if possible #95898

wants to merge 4 commits into from

Conversation

voznesenskym
Copy link
Contributor

@voznesenskym voznesenskym commented Mar 2, 2023 •
edited by pytorch-bot bot

@voznesenskym
[RFC] Use original arg names if possible
a89506f 
Use graphargs

rm

[ghstack-poisoned]
@pytorch-bot
Copy link

pytorch-bot bot commented Mar 2, 2023 •
edited

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/95898

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures

As of commit 4ad3cb1:
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

voznesenskym added a commit that referenced this pull request Mar 2, 2023
@voznesenskym
[RFC] Use original arg names if possible
9c6cd18 
Use graphargs

rm

ghstack-source-id: d4de6ed5c6c92ab09bd0068ab97f55a6aabfa099
Pull Request resolved: #95898
@voznesenskym voznesenskym marked this pull request as draft March 2, 2023 19:07
@albanD albanD removed their request for review March 2, 2023 21:14
@voznesenskym
Update on "[RFC] Use original arg names if possible"
caf029b 
Use graphargs

rm

cc soumith yanboliang penguinwu anijain2305 EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx desertfire

[ghstack-poisoned]
voznesenskym added a commit that referenced this pull request Mar 3, 2023
@voznesenskym
[RFC] Use original arg names if possible
227dfec 
Use graphargs

rm

ghstack-source-id: 0c396cfde3f59dc9517fc61bd646f3ea517f0763
Pull Request resolved: #95898

stargs

test
@voznesenskym voznesenskym marked this pull request as ready for review March 3, 2023 23:08
@voznesenskym voznesenskym changed the title [RFC] Use original arg names if possible Use original arg names if possible Mar 3, 2023
@github-actions github-actions bot requested a review from albanD March 3, 2023 23:08
@voznesenskym voznesenskym added the topic: not user facing topic category label Mar 3, 2023
suo
suo approved these changes Mar 3, 2023
View reviewed changes
test/dynamo/test_export.py Outdated

mod, input_tensor = BasicModule(), torch.randn(2, 3)
gm, guard = torch._dynamo.export(mod, input_tensor, aten_graph=True)
print(gm.graph)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stray prints

also, this should probably be testing something about the graphs input names?
And can we test the weird corner cases: *args, **kwargs, etc.

@voznesenskym
Update on "Use original arg names if possible"
ddb2e53 
Use graphargs

rm

cc soumith yanboliang penguinwu anijain2305 EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx desertfire

[ghstack-poisoned]
voznesenskym added a commit that referenced this pull request Mar 5, 2023
@voznesenskym
[RFC] Use original arg names if possible
946a4ee 
Use graphargs

rm

ghstack-source-id: f8fa0c715a92ecb90432b6066c66c77b93342439
Pull Request resolved: #95898

stargs

test

Test fix
@voznesenskym
Copy link
Contributor Author

@pytorchbot merge

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Mar 6, 2023
@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging
Check the merge workflow status
here

@pytorchmergebot
Copy link
Collaborator

Merge failed

Reason: Command git -C /home/runner/work/pytorch/pytorch cherry-pick -x 946a4eef85636e3475b87c47694487592d148e8d returned non-zero exit code 1

Auto-merging test/dynamo/test_export.py
CONFLICT (content): Merge conflict in test/dynamo/test_export.py
Auto-merging torch/_dynamo/eval_frame.py
CONFLICT (content): Merge conflict in torch/_dynamo/eval_frame.py
error: could not apply 946a4eef856... [RFC] Use original arg names if possible
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".
Details for Dev Infra team Raised by workflow job

@voznesenskym
Update on "Use original arg names if possible"
4ad3cb1 
Use graphargs

rm

cc soumith yanboliang penguinwu anijain2305 EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx desertfire

[ghstack-poisoned]
voznesenskym added a commit that referenced this pull request Mar 6, 2023
@voznesenskym
[RFC] Use original arg names if possible
ce642cd 
Use graphargs

rm

ghstack-source-id: a4f5274425e8529e993414b17a83386415432b54
Pull Request resolved: #95898

stargs

test

Test fix
@voznesenskym
Copy link
Contributor Author

@pytorchbot merge

@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging
Check the merge workflow status
here

@BowenBao
Copy link
Collaborator

BowenBao commented Mar 6, 2023

Got a PR that tries to do the same thing lol. @voznesenskym please take a look if you haven't already. #95851

ydwu4 pushed a commit to ydwu4/pytorch that referenced this pull request Mar 10, 2023
@voznesenskym @ydwu4
Use original arg names if possible (pytorch#95898)
654accd 
Use graphargs

rm

Pull Request resolved: pytorch#95898
Approved by: https://github.com/suo
cyyever pushed a commit to cyyever/pytorch_private that referenced this pull request Mar 12, 2023
@voznesenskym @cyyever
Use original arg names if possible (#95898)
f1832ab 
Use graphargs

rm

Pull Request resolved: pytorch/pytorch#95898
Approved by: https://github.com/suo
cyyever pushed a commit to cyyever/pytorch_private that referenced this pull request Mar 12, 2023
@voznesenskym @cyyever
Use original arg names if possible (#95898)
c931254 
Use graphargs

rm

Pull Request resolved: pytorch/pytorch#95898
Approved by: https://github.com/suo
ydwu4 added a commit to ydwu4/pytorch that referenced this pull request Mar 13, 2023
@ydwu4
Use original arg names if possible (pytorch#95898)
aaa668a 
Use graphargs

rm

Pull Request resolved: pytorch#95898
Approved by: https://github.com/suo
@facebook-github-bot facebook-github-bot deleted the gh/voznesenskym/74/head branch June 8, 2023 19:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@suo suo suo approved these changes

@antoniojkim antoniojkim Awaiting requested review from antoniojkim

@bdhirsh bdhirsh Awaiting requested review from bdhirsh

@Chillee Chillee Awaiting requested review from Chillee

@ezyang ezyang Awaiting requested review from ezyang

@jbschlosser jbschlosser Awaiting requested review from jbschlosser

@miladm miladm Awaiting requested review from miladm

@SherlockNoMad SherlockNoMad Awaiting requested review from SherlockNoMad

@wconstab wconstab Awaiting requested review from wconstab

@albanD albanD Awaiting requested review from albanD

Assignees
No one assigned
Labels
ciflow/inductor ciflow/trunk Trigger trunk jobs on your pull request Merged module: dynamo topic: not user facing topic category
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@voznesenskym @pytorchmergebot @BowenBao @suo