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

Refractor export code for non/strict path #126354

Closed
wants to merge 1 commit into from
Closed

Refractor export code for non/strict path #126354

wants to merge 1 commit into from
+293 −257

Conversation

jiashenC
Copy link
Contributor

  • Remove redundant code
  • Move strict and non-strict export code to their own functions

@pytorch-bot PyTorch Bot
Copy link

pytorch-bot bot commented May 15, 2024 •
edited

🔗 Helpful Links

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

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

❌ 1 New Failure

As of commit 76dc1d2 with merge base d74c89f (image):

NEW FAILURE - The following job has failed:

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

angelayi
angelayi reviewed May 16, 2024
View reviewed changes
Copy link
Contributor

@angelayi angelayi left a comment

Choose a reason for hiding this comment

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

looks like a good start!

torch/export/_trace.py
with _wrap_submodules(
wrapped_mod, new_preserved_call_signatures, module_call_specs
):
gm, sig = aot_export(wrapped_mod, args, kwargs=kwargs, **flags)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you can move the log after this statement.

torch/export/_trace.py
Comment on lines +1420 to +1421
if strict:
log.debug("Exported program from AOTAutograd:\n%s", exported_program)
Copy link
Contributor

Choose a reason for hiding this comment

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

This log is invalid now. Instead can we put a log at the comment I put above?

torch/export/_trace.py Show resolved Hide resolved
@jiashenC jiashenC closed this May 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@angelayi angelayi angelayi left review comments

@avikchaudhuri avikchaudhuri Awaiting requested review from avikchaudhuri avikchaudhuri will be requested when the pull request is marked ready for review avikchaudhuri is a code owner

@gmagogsfm gmagogsfm Awaiting requested review from gmagogsfm gmagogsfm will be requested when the pull request is marked ready for review gmagogsfm is a code owner

@tugsbayasgalan tugsbayasgalan Awaiting requested review from tugsbayasgalan tugsbayasgalan will be requested when the pull request is marked ready for review tugsbayasgalan is a code owner

@zhxchen17 zhxchen17 Awaiting requested review from zhxchen17 zhxchen17 will be requested when the pull request is marked ready for review zhxchen17 is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@jiashenC @angelayi