Skip to content

[dynamo][guards-log] Add debug msg for nn_module_guards only when log is enabled#110167

Closed
anijain2305 wants to merge 1 commit intogh/anijain2305/136/basefrom
gh/anijain2305/136/head
Closed

[dynamo][guards-log] Add debug msg for nn_module_guards only when log is enabled#110167
anijain2305 wants to merge 1 commit intogh/anijain2305/136/basefrom
gh/anijain2305/136/head

Conversation

@anijain2305
Copy link
Copy Markdown
Contributor

@anijain2305 anijain2305 commented Sep 27, 2023

Stack from ghstack (oldest at bottom):

I did not do any benchmarks, but there could be a small overhead of creating the debug_msg. Adding debug_msg only when guards log is enabled.

cc @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @Xia-Weiwen @wenzhe-nrv @jiayisunx @chenyang78 @aakhundov @kadeng

@pytorch-bot
Copy link
Copy Markdown

pytorch-bot bot commented Sep 27, 2023

🔗 Helpful Links

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

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

⏳ No Failures, 1 Pending

As of commit d8f8895 with merge base d0f82cd (image):
💚 Looks good so far! There are no failures yet. 💚

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

anijain2305 added a commit that referenced this pull request Sep 27, 2023
… is enabled

ghstack-source-id: b601f51
Pull Request resolved: #110167
@anijain2305 anijain2305 requested a review from ezyang September 27, 2023 16:02

# debug_msg is only for debugging help and goes to kwargs of guard call,
# which is ignored.
self._produce_guard_code(guard, [f'{name}({ref}, debug_msg="{g}")'])
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we make this more symmetric with how regular guard debugging? Specifically, pass the debug message to produce_guard_code, and propagate it to the debugging logic later

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Will look more into this. Let me get this one in to avoid perf regressions if any.

@anijain2305
Copy link
Copy Markdown
Contributor Author

@pytorchbot merge

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Sep 27, 2023
@anijain2305 anijain2305 added topic: not user facing topic category and removed ciflow/trunk Trigger trunk jobs on your pull request labels Sep 27, 2023
@pytorchmergebot
Copy link
Copy Markdown
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
Copy Markdown
Collaborator

Merge failed

Reason: 1 jobs have failed, first few of them are: trunk / linux-focal-rocm5.6-py3.8 / test (default, 2, 3, linux.rocm.gpu)

Details for Dev Infra team Raised by workflow job

@anijain2305
Copy link
Copy Markdown
Contributor Author

@pytorchbot merge -f "unrelated rocm failure"

@pytorchmergebot
Copy link
Copy Markdown
Collaborator

Merge started

Your change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes). Please use -f as last resort and instead consider -i/--ignore-current to continue the merge ignoring current failures. This will allow currently pending tests to finish and report signal before the merge.

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

@facebook-github-bot facebook-github-bot deleted the gh/anijain2305/136/head branch October 1, 2023 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants