Skip to content

have a common interface to extract metadata from SizeNodes#78088

Closed
Krovatkin wants to merge 1 commit intopytorch:masterfrom
Krovatkin:krovatkin/dim_int
Closed

have a common interface to extract metadata from SizeNodes#78088
Krovatkin wants to merge 1 commit intopytorch:masterfrom
Krovatkin:krovatkin/dim_int

Conversation

@Krovatkin
Copy link
Copy Markdown
Contributor

Fixes #ISSUE_NUMBER

@facebook-github-bot
Copy link
Copy Markdown
Contributor

facebook-github-bot commented May 23, 2022

🔗 Helpful links

✅ No Failures (0 Pending)

As of commit 14971d4 (more details on the Dr. CI page):

Expand to see more

💚 💚 Looks good so far! There are no failures yet. 💚 💚


This comment was automatically generated by Dr. CI (expand for details).

Please report bugs/suggestions to the (internal) Dr. CI Users group.

Click here to manually regenerate this comment.

@Krovatkin Krovatkin force-pushed the krovatkin/dim_int branch from 34029fe to e8497b8 Compare June 1, 2022 16:48
@Krovatkin Krovatkin changed the title have a common interface to extract metadata from SizeNodes [WIP] have a common interface to extract metadata from SizeNodes Jun 1, 2022
@Krovatkin Krovatkin requested review from JackCaoG and wconstab June 1, 2022 18:05
Comment thread torch/csrc/lazy/ts_backend/dynamic_ir.h Outdated
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Ah, this is neat. How are we going to represent a common SizeNode in the LTC code path? Is it going to be shared_ptr< DimensionNode> ?

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.

Correct! I think I should just add a test. Initially, I just wanted to get @miladm to try it out w/ his PR :)

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.

added a test

Copy link
Copy Markdown
Collaborator

@JackCaoG JackCaoG left a comment

Choose a reason for hiding this comment

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

Thanks!

Copy link
Copy Markdown
Contributor

@wconstab wconstab left a comment

Choose a reason for hiding this comment

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

lgtm, thanks nick!

Comment thread test/cpp/lazy/test_ir.cpp Outdated
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.

I thought we are supposed to use <> inports.

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.

yes we are... that's clang-tidy :(

@miladm
Copy link
Copy Markdown
Collaborator

miladm commented Jun 14, 2022

FIXES #77880

@Krovatkin Krovatkin force-pushed the krovatkin/dim_int branch 2 times, most recently from bcc6f68 to 579d295 Compare June 14, 2022 21:27
have a common interface to extract metadata from SizeNodes
@Krovatkin Krovatkin force-pushed the krovatkin/dim_int branch from e8ded00 to 14971d4 Compare June 14, 2022 22:50
@Krovatkin
Copy link
Copy Markdown
Contributor Author

@pytorchbot merge

@pytorchmergebot
Copy link
Copy Markdown
Collaborator

@pytorchbot successfully started a merge job. Check the current status here

@github-actions
Copy link
Copy Markdown
Contributor

Hey @Krovatkin.
You've committed this PR, but it does not have both a 'release notes: ...' and 'topics: ...' label. Please add one of each to the PR. The 'release notes: ...' label should represent the part of PyTorch that this PR changes (fx, autograd, distributed, etc) and the 'topics: ...' label should represent the kind of PR it is (not user facing, new feature, bug fix, perf improvement, etc). The list of valid labels can be found here for the 'release notes: ...' and here for the 'topics: ...'.
For changes that are 'topic: not user facing' there is no need for a release notes label.

facebook-github-bot pushed a commit that referenced this pull request Jun 20, 2022
…78088)

Summary:
Fixes #ISSUE_NUMBER

Pull Request resolved: #78088
Approved by: https://github.com/JackCaoG, https://github.com/wconstab

Test Plan: contbuild & OSS CI, see https://hud.pytorch.org/commit/pytorch/pytorch/83e575c51024746b9f5174d1d924657891bb99e9

Reviewed By: malfet

Differential Revision: D37208289

Pulled By: Krovatkin

fbshipit-source-id: c2bb988b391d0e849079afef6573fbb3ae324785
laurentdupin pushed a commit to laurentdupin/pytorch that referenced this pull request Apr 25, 2026
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.

7 participants