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.

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

Node.stack_trace should have innermost frame last #95592

Closed

Conversation

SherlockNoMad
Copy link
Contributor

@SherlockNoMad SherlockNoMad commented Feb 27, 2023

@pytorch-bot
Copy link

pytorch-bot bot commented Feb 27, 2023

🔗 Helpful Links

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

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

✅ No Failures

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

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

@pytorch-bot pytorch-bot bot added release notes: fx release notes category labels Feb 27, 2023
SherlockNoMad added a commit that referenced this pull request Feb 27, 2023
ghstack-source-id: ee9ddd25d268e431d7117976eade9b3b54cc4487
Pull Request resolved: #95592
@SherlockNoMad SherlockNoMad changed the title Fode.sstack_trace should have innermost frame last Node.stack_trace should have innermost frame last Feb 27, 2023
@@ -180,9 +180,9 @@ def create_proxy(self, kind: str, target: Target, args: Tuple[Any, ...], kwargs:
if self.record_stack_traces and not proxy.node.stack_trace:
user_frame = self._find_user_frame()
if user_frame:
walk_stack_gen = traceback.walk_stack(user_frame)
summary = traceback.StackSummary.extract(walk_stack_gen) # type: ignore[arg-type]
Copy link
Contributor

Choose a reason for hiding this comment

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

interesting, I wonder why we did it this way originally

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure... traceback.extract_stack(frame) would reverse the stack for user.

torch/fx/node.py Outdated Show resolved Hide resolved
# iterate backwards to find the first line that starts
# with 'File '
idx = len(lines) - 1
while idx >= 0:
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: use a range loop here? for idx in range(len(lines)-1, -1, -1)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, I rewrote to simplify the code.

torch/fx/graph.py Outdated Show resolved Hide resolved
Copy link
Contributor

@ezyang ezyang left a comment

Choose a reason for hiding this comment

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

Thanks!

Both fx.Tracer and Dynamo should store node.stack_trace in the "innermost frame last" order. 




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

[ghstack-poisoned]
SherlockNoMad added a commit that referenced this pull request Feb 27, 2023
ghstack-source-id: 8a1b4d307733ab7b9982a3c483a22960323abbe7
Pull Request resolved: #95592
@SherlockNoMad
Copy link
Contributor Author

@pytorchbot merge

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Feb 27, 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: 1 jobs have failed, first few of them are: inductor / linux-focal-cpu-py3.8-gcc7-inductor / test (inductor_timm_cpu_accuracy, 2, 2, linux.4xlarge)

Details for Dev Infra team Raised by workflow job

@SherlockNoMad
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

cyyever pushed a commit to cyyever/pytorch_private that referenced this pull request Mar 2, 2023
Both fx.Tracer and Dynamo should store node.stack_trace in the "innermost frame last" order.

Pull Request resolved: pytorch/pytorch#95592
Approved by: https://github.com/ezyang
cyyever pushed a commit to cyyever/pytorch_private that referenced this pull request Mar 5, 2023
Both fx.Tracer and Dynamo should store node.stack_trace in the "innermost frame last" order.

Pull Request resolved: pytorch/pytorch#95592
Approved by: https://github.com/ezyang
cyyever pushed a commit to cyyever/pytorch_private that referenced this pull request Mar 5, 2023
Both fx.Tracer and Dynamo should store node.stack_trace in the "innermost frame last" order.

Pull Request resolved: pytorch/pytorch#95592
Approved by: https://github.com/ezyang
pruthvistony added a commit to ROCm/pytorch that referenced this pull request May 2, 2023
@facebook-github-bot facebook-github-bot deleted the gh/SherlockNoMad/112/head branch June 8, 2023 14:51
jhavukainen pushed a commit to kulinseth/pytorch that referenced this pull request Mar 15, 2024
Both fx.Tracer and Dynamo should store node.stack_trace in the "innermost frame last" order.

Pull Request resolved: pytorch#95592
Approved by: https://github.com/ezyang
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ciflow/inductor ciflow/trunk Trigger trunk jobs on your pull request Merged module: dynamo release notes: fx release notes category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants