add torch.meshgrid() OpInfo #62720

dagitses · 2021-08-04T13:37:01Z

Fixes #62719

facebook-github-bot · 2021-08-04T13:37:07Z

🔗 Helpful links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/62720
📄 Preview docs built from this PR

💊 CI failures summary and remediations

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

1/1 failures possibly* introduced in this PR
- 1/1 non-scanned failure(s)

ci.pytorch.org: 1 failed

Failed: pr/pytorch-linux-bionic-rocm4.2-py3.6

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

Follow this link to opt-out of these comments for your Pull Requests.

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

Click here to manually regenerate this comment.

mruberry · 2021-08-11T16:12:23Z

torch/testing/_internal/common_methods_invocations.py



+def sample_inputs_meshgrid(
+        calling_convention: str) -> Callable[[OpInfo, torch.device, torch.dtype, bool], List[SampleInput]]:


Adding all this type information looks time-consuming, did the system make you do it? We should probably stop linting this file if so

I don't remember if it made me do it. It seems unlikely given that most of the file is not type annotated.

torch/testing/_internal/common_methods_invocations.py

mruberry · 2021-08-11T16:15:06Z

torch/testing/_internal/common_methods_invocations.py

+
+    def sample_inputs(op_info: OpInfo, device: torch.device, dtype: torch.dtype,
+                      requires_grad: bool) -> List[SampleInput]:
+        Z = ()  # note: torch.rand(()) yields a random 0D tensor


Z seems like a fine addition to the S M and L defined at the global scope

done. i have some concern that Z is not necessarily interchangeable with the other sizes, since it represents a full shape, whereas sometimes S et al. are used as the length of a single dimension.

Fair point, but from that perspective I'd suggest simply not defining it and always writing (), then, because that concern is also a local concern

True, it is a local concern. S here is also a local, not the value from above. Do you think that having two local constants S and Z here would be inappropriate?

S = torch.Size([3]) Z = torch.Size([])

I'd prefer not shadowing the global for readability, especially not shadowing it with a very similar value

changed to locals named SCALAR and VECTOR

torch/testing/_internal/common_methods_invocations.py

mruberry

Hey @dagitses! Overall looks good. I made some notes about possibly simplifying the sample inputs.

mruberry · 2021-08-16T17:26:52Z

torch/testing/_internal/common_methods_invocations.py

+            'Unsupported variant, must be one of {"variadic", "list"}. '
+            f'Got "{variant}".')
+
+    SCALAR = torch.Size([])


I'm not sure these are really improving readability here but I guess it's OK

mruberry

Cool!

facebook-github-bot · 2021-08-16T17:48:47Z

@dagitses has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

facebook-github-bot · 2021-08-17T02:20:12Z

@dagitses has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

facebook-github-bot · 2021-08-17T02:29:01Z

@dagitses has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

facebook-github-bot · 2021-08-17T02:30:47Z

@dagitses has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

facebook-github-bot · 2021-08-17T11:05:40Z

@dagitses merged this pull request in cae5ddc.

facebook-github-bot added the cla signed label Aug 4, 2021

dagitses force-pushed the dagitses/#62719/meshgrid-opinfo branch 3 times, most recently from c885221 to 4e8e658 Compare August 4, 2021 20:30

dagitses mentioned this pull request Aug 4, 2021

add torch.meshgrid() OpInfo #62523

Closed

dagitses force-pushed the dagitses/#62719/meshgrid-opinfo branch 6 times, most recently from e5f5daa to 897ba90 Compare August 9, 2021 17:13

dagitses marked this pull request as ready for review August 9, 2021 17:16

dagitses force-pushed the dagitses/#62719/meshgrid-opinfo branch 4 times, most recently from c3672cf to 3d1b67a Compare August 10, 2021 00:48

dagitses changed the base branch from master to dagitses/meshgrid-doc August 10, 2021 01:53

dagitses force-pushed the dagitses/#62719/meshgrid-opinfo branch from 3d1b67a to 4ff55ce Compare August 11, 2021 14:42

dagitses requested review from H-Huang, albanD, cbalioglu, driazati, janeyx99, mingzhe09088, mrshenli, pritamdamania87, rohan-varma, seemethere and wayi1 as code owners August 11, 2021 14:42

mruberry requested review from mruberry and removed request for janeyx99, mingzhe09088 and zhaojuanmao August 11, 2021 16:11

mruberry reviewed Aug 11, 2021

View reviewed changes

torch/testing/_internal/common_methods_invocations.py Outdated Show resolved Hide resolved

mruberry reviewed Aug 11, 2021

View reviewed changes

torch/testing/_internal/common_methods_invocations.py Show resolved Hide resolved

mruberry reviewed Aug 11, 2021

View reviewed changes

torch/testing/_internal/common_methods_invocations.py Outdated Show resolved Hide resolved

mruberry reviewed Aug 11, 2021

View reviewed changes

dagitses force-pushed the dagitses/#62719/meshgrid-opinfo branch 4 times, most recently from 95bbdc6 to 65ddedf Compare August 14, 2021 09:53

mruberry reviewed Aug 16, 2021

View reviewed changes

mruberry self-requested a review August 16, 2021 17:28

mruberry approved these changes Aug 16, 2021

View reviewed changes

dagitses force-pushed the dagitses/#62719/meshgrid-opinfo branch from 65ddedf to 5c980fe Compare August 16, 2021 17:44

dagitses force-pushed the dagitses/#62719/meshgrid-opinfo branch from 5c980fe to fb4ef92 Compare August 17, 2021 02:28

add torch.meshgrid() OpInfo

aa6973e

dagitses force-pushed the dagitses/#62719/meshgrid-opinfo branch from fb4ef92 to aa6973e Compare August 17, 2021 02:30

facebook-github-bot closed this in cae5ddc Aug 17, 2021

facebook-github-bot added the Merged label Aug 17, 2021

dagitses deleted the dagitses/#62719/meshgrid-opinfo branch August 17, 2021 16:55



		def sample_inputs_meshgrid(
		calling_convention: str) -> Callable[[OpInfo, torch.device, torch.dtype, bool], List[SampleInput]]:

add torch.meshgrid() OpInfo #62720

add torch.meshgrid() OpInfo #62720

Uh oh!

Conversation

dagitses commented Aug 4, 2021

Uh oh!

facebook-github-bot commented Aug 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful links

💊 CI failures summary and remediations

ci.pytorch.org: 1 failed

Uh oh!

mruberry Aug 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mruberry left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mruberry left a comment

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Aug 16, 2021

Uh oh!

facebook-github-bot commented Aug 17, 2021

Uh oh!

facebook-github-bot commented Aug 17, 2021

Uh oh!

facebook-github-bot commented Aug 17, 2021

Uh oh!

facebook-github-bot commented Aug 17, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

facebook-github-bot commented Aug 4, 2021 •

edited

Loading

mruberry Aug 11, 2021 •

edited

Loading