OpInfo: Add test that sample_inputs_func returns a generator #84567

peterbell10 · 2022-09-06T14:42:46Z

Stack from ghstack (oldest at bottom):

-> OpInfo: Add test that sample_inputs_func returns a generator #84567

This also includes a small list exception for single element lists since none of the memory usage or performance implications of lists apply there.

[ghstack-poisoned]

facebook-github-bot · 2022-09-06T14:42:57Z

🔗 Helpful links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/84567
✖️ Python docs build was skipped
✖️ C++ docs build was skipped
❓Need help or want to give feedback on the CI? Visit our office hours

✅ No Failures (31 Pending)

As of commit a8fc80a (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.

[ghstack-poisoned]

pytorch-bot · 2022-09-08T14:19:04Z

🔗 Helpful Links

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

📄 Preview Python docs built from this PR
📄 Preview C++ docs built from this PR
❓ Need help or want to give feedback on the CI? Visit our office hours

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

✅ No Failures

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

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

[ghstack-poisoned]

This does include a small list exception for less than 4 elements, but that will be tightened to 1 element in a follow-up PR. [ghstack-poisoned]

mruberry · 2022-10-17T17:49:02Z

test/test_testing.py

        self.assertEqual(s2.name, "foo")


+class TestOpInfoSampleFunctions(TestCase):


Add a comment describing this class

mruberry · 2022-10-17T17:52:30Z

test/test_testing.py

+        else:
+            self.assertIsInstance(item, Generator)
+
+    @ops(op_db, dtypes=OpDTypes.any_one)


These tests make a lot of sense (comments describing each one, please!)

Should we consider adding these checks to the tests in test_ops.py that exercise these generators, instead? That's not conceptually ideal because then those tests are testing multiple things (they probably already are), but it would reduce duplicative work in the test suite, which does a lot already. Or are these new tests so fast we shouldn't be concerned about their speed?

An example of a similar case is the test for "input" tensors having no grad history, which is in the noncontiguous samples test:

pytorch/test/test_ops.py

Line 503 in c27a517

# Verifies sample input tensors should have no grad or history

cc @lezcano

A great thing to have would be a test that tests OpInfo samples themselves, and have all these tests grouped there. Until then, if this test is fast enough, it's probably good enough.

On my machine

time pytest test_testing.py -k TestOpInfoSampleFunctions

takes less than 10 seconds even on a build with CUDA. So I'm not particularly concerned.

On the other hand if we combine these checks into an existing test, then the test is more likely to be skipped for no good reason.

This also includes a small list exception for single element lists. ghstack-source-id: 9c6f73b Pull Request resolved: pytorch#84567

This also includes a small list exception for single element lists since none of the memory usage or performance implications of lists apply there. [ghstack-poisoned]

This also includes a small list exception for single element lists. ghstack-source-id: 38914eb Pull Request resolved: #84567

lezcano

Good stuff.

mruberry

Timing looks good, but see notes requesting additional comments and the lint failure may be real!

This also includes a small list exception for single element lists since none of the memory usage or performance implications of lists apply there. [ghstack-poisoned]

This also includes a small list exception for single element lists. ghstack-source-id: 6ef0f06 Pull Request resolved: #84567

…rns a generator" This also includes a small list exception for single element lists since none of the memory usage or performance implications of lists apply there. [ghstack-poisoned]

This also includes a small list exception for single element lists. ghstack-source-id: cc6164a Pull Request resolved: #84567

peterbell10 · 2022-10-20T21:53:34Z

@pytorchbot merge

pytorchmergebot · 2022-10-20T21:55:00Z

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 · 2022-10-20T21:55:05Z

Merge failed

Reason: The following mandatory check(s) failed (Rule Core Reviewers):

pull

Dig deeper by viewing the failures on hud

Details for Dev Infra team

Raised by workflow job

This also includes a small list exception for single element lists since none of the memory usage or performance implications of lists apply there. [ghstack-poisoned]

This also includes a small list exception for single element lists. ghstack-source-id: 4d8f9e9 Pull Request resolved: #84567

peterbell10 · 2022-10-21T14:55:12Z

@pytorchbot merge

pytorchmergebot · 2022-10-21T14:58:24Z

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

…#84567) This also includes a small list exception for single element lists since none of the memory usage or performance implications of lists apply there. Pull Request resolved: pytorch#84567 Approved by: https://github.com/lezcano, https://github.com/mruberry

OpInfo: Add test that sample_inputs_func returns a generator

57b7843

[ghstack-poisoned]

facebook-github-bot added the cla signed label Sep 6, 2022

peterbell10 marked this pull request as draft September 6, 2022 14:46

pytorchbot added the open source label Sep 6, 2022

Update on "OpInfo: Add test that sample_inputs_func returns a generator"

deb0c75

[ghstack-poisoned]

peterbell10 mentioned this pull request Sep 7, 2022

OpInfo: Extend generator testing to reference and error input funcs #84631

Closed

peterbell10 added 2 commits September 7, 2022 16:23

Update on "OpInfo: Add test that sample_inputs_func returns a generator"

6c5e877

[ghstack-poisoned]

Update on "OpInfo: Add test that sample_inputs_func returns a generator"

a665c22

[ghstack-poisoned]

peterbell10 added 3 commits September 7, 2022 19:22

Update on "OpInfo: Add test that sample_inputs_func returns a generator"

65d79bb

[ghstack-poisoned]

Update on "OpInfo: Add test that sample_inputs_func returns a generator"

a8fc80a

[ghstack-poisoned]

Rebase on "OpInfo: Add test that sample_inputs_func returns a generator"

6af1e70

[ghstack-poisoned]

peterbell10 mentioned this pull request Sep 8, 2022

OpInfo: Prevent clamp sample inputs from sharing tensors #84696

Closed

Update on "OpInfo: Add test that sample_inputs_func returns a generator"

bb7b053

[ghstack-poisoned]

pytorch-bot bot added the topic: not user facing topic category label Sep 20, 2022

Update on "OpInfo: Add test that sample_inputs_func returns a generator"

29220cc

[ghstack-poisoned]

peterbell10 added 4 commits September 22, 2022 15:11

Update on "OpInfo: Add test that sample_inputs_func returns a generator"

4e220a0

[ghstack-poisoned]

Update on "OpInfo: Add test that sample_inputs_func returns a generator"

99282e9

[ghstack-poisoned]

Update on "OpInfo: Add test that sample_inputs_func returns a generator"

8f2c34b

[ghstack-poisoned]

Update on "OpInfo: Add test that sample_inputs_func returns a generator"

80054d1

This does include a small list exception for less than 4 elements, but that will be tightened to 1 element in a follow-up PR. [ghstack-poisoned]

peterbell10 marked this pull request as ready for review October 17, 2022 12:29

peterbell10 added the module: testing Issues related to the torch.testing module (not tests) label Oct 17, 2022

peterbell10 requested a review from mruberry October 17, 2022 12:30

mruberry reviewed Oct 17, 2022

View reviewed changes

Update on "OpInfo: Add test that sample_inputs_func returns a generator"

4e29861

This also includes a small list exception for single element lists since none of the memory usage or performance implications of lists apply there. [ghstack-poisoned]

peterbell10 added a commit that referenced this pull request Oct 17, 2022

OpInfo: Add test that sample_inputs_func returns a generator

a8c4f17

This also includes a small list exception for single element lists. ghstack-source-id: 38914eb Pull Request resolved: #84567

lezcano approved these changes Oct 18, 2022

View reviewed changes

pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Oct 18, 2022

mruberry approved these changes Oct 18, 2022

View reviewed changes

Update on "OpInfo: Add test that sample_inputs_func returns a generator"

e1a907c

This also includes a small list exception for single element lists since none of the memory usage or performance implications of lists apply there. [ghstack-poisoned]

peterbell10 added a commit that referenced this pull request Oct 19, 2022

OpInfo: Add test that sample_inputs_func returns a generator

4f6e7dc

This also includes a small list exception for single element lists. ghstack-source-id: 6ef0f06 Pull Request resolved: #84567

Fix merge conflicts on "OpInfo: Add test that sample_inputs_func retu…

fe68eb5

…rns a generator" This also includes a small list exception for single element lists since none of the memory usage or performance implications of lists apply there. [ghstack-poisoned]

peterbell10 added a commit that referenced this pull request Oct 20, 2022

OpInfo: Add test that sample_inputs_func returns a generator

9842bed

This also includes a small list exception for single element lists. ghstack-source-id: cc6164a Pull Request resolved: #84567

Update on "OpInfo: Add test that sample_inputs_func returns a generator"

7652954

This also includes a small list exception for single element lists since none of the memory usage or performance implications of lists apply there. [ghstack-poisoned]

peterbell10 added a commit that referenced this pull request Oct 21, 2022

OpInfo: Add test that sample_inputs_func returns a generator

41fe919

This also includes a small list exception for single element lists. ghstack-source-id: 4d8f9e9 Pull Request resolved: #84567

pytorchmergebot added the Merged label Oct 21, 2022

pytorchmergebot closed this in 1285542 Oct 21, 2022

facebook-github-bot deleted the gh/peterbell10/417/head branch June 8, 2023 18:20

		self.assertEqual(s2.name, "foo")


		class TestOpInfoSampleFunctions(TestCase):

OpInfo: Add test that sample_inputs_func returns a generator #84567

OpInfo: Add test that sample_inputs_func returns a generator #84567

Uh oh!

Conversation

peterbell10 commented Sep 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

facebook-github-bot commented Sep 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful links

✅ No Failures (31 Pending)

Uh oh!

pytorch-bot bot commented Sep 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful Links

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

✅ No Failures

Uh oh!

mruberry Oct 17, 2022

Choose a reason for hiding this comment

Uh oh!

mruberry Oct 17, 2022

Choose a reason for hiding this comment

Uh oh!

lezcano Oct 17, 2022 • 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!

peterbell10 Oct 18, 2022

Choose a reason for hiding this comment

Uh oh!

lezcano Oct 18, 2022

Choose a reason for hiding this comment

Uh oh!

lezcano left a comment

Choose a reason for hiding this comment

Uh oh!

mruberry left a comment

Choose a reason for hiding this comment

Uh oh!

peterbell10 commented Oct 20, 2022

Uh oh!

pytorchmergebot commented Oct 20, 2022

Merge started

Uh oh!

pytorchmergebot commented Oct 20, 2022

Merge failed

Uh oh!

peterbell10 commented Oct 21, 2022

Uh oh!

pytorchmergebot commented Oct 21, 2022

Merge started

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

peterbell10 commented Sep 6, 2022 •

edited

Loading

facebook-github-bot commented Sep 6, 2022 •

edited

Loading

pytorch-bot bot commented Sep 8, 2022 •

edited

Loading

lezcano Oct 17, 2022 •

edited

Loading