Add exhaustive testing to ValueRanges, fix bugs #94939

ezyang · 2023-02-15T21:47:27Z

Stack from ghstack (oldest at bottom):

Since I didn't want to deal with nondeterministic tests, I went the exhaustive testing route for a fixed list of constants to look at. The tests generate random ranges, propagate the range through the function, and then pick elements in the range and check that the result on the operation is in the resulting range. This caught bugs in log, sqrt and pow.

My resolution for pow was a little special, because I had trouble figuring out the correct semantics under all inputs domains. Instead, I picked two input domains (pow on two point ranges, and pow where exponent is known) and only implemented those. Everything else we give up. I think this is unlikely to affect perf.

Signed-off-by: Edward Z. Yang ezyang@meta.com

cc @gujinghui @PenghuiCheng @XiaobingSuper @jianyuh @jgong5 @mingfeima @sanchitintel @ashokei @jingxu10 @min-jean-cho @yanbing-j @Guobing-Chen @Xia-Weiwen

Signed-off-by: Edward Z. Yang <ezyang@meta.com> [ghstack-poisoned]

pytorch-bot · 2023-02-15T21:47:29Z

🔗 Helpful Links

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

📄 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 the bot commands wiki or our office hours

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

✅ No Failures

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

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

Signed-off-by: Edward Z. Yang <ezyangmeta.com> ghstack-source-id: 6a3d0236bcf4ad1e8b8070a519c2f802d9105c62 Pull Request resolved: #94939

lezcano · 2023-02-15T22:37:29Z

torch/utils/_sympy/value_ranges.py

+            # This is fairly difficult to analyze, so give up for anything
+            # complicated


Right, I see we were missing a couple cases here, yep.

For future reference, it's probably enough to analyse the signs of a and b separately and have those 4 cases, but yeah, probably not worth the effort.

PRs welcome! The problem I had was that you also have different regimes for a < 1 vs > 1 vs = 1. PAIN

I'll submit a PR once this one's merged.

torch/utils/_sympy/value_ranges.py

eellison

nice !

eellison · 2023-02-16T00:04:46Z

test/test_value_ranges.py

@@ -119,15 +122,45 @@ def ceil(x):
        return math.ceil(x)


+def valid_unary(fn, v):


we could also run the reference first and see if it throws, then skip

Still good to avoid trying to eval something silly like 2 ** 10000 which sympy will do symbolically lol

test/test_value_ranges.py

torch/utils/_sympy/value_ranges.py

ezyang · 2023-02-16T13:51:44Z

@pytorchbot merge

pytorchmergebot · 2023-02-16T13:54:57Z

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

Since I didn't want to deal with nondeterministic tests, I went the exhaustive testing route for a fixed list of constants to look at. The tests generate random ranges, propagate the range through the function, and then pick elements in the range and check that the result on the operation is in the resulting range. This caught bugs in log, sqrt and pow. My resolution for pow was a little special, because I had trouble figuring out the correct semantics under all inputs domains. Instead, I picked two input domains (pow on two point ranges, and pow where exponent is known) and only implemented those. Everything else we give up. I think this is unlikely to affect perf. Signed-off-by: Edward Z. Yang <ezyangmeta.com> [ghstack-poisoned]

pytorchmergebot · 2023-02-16T14:05:07Z

Merge failed

Reason: New commits were pushed while merging. Please rerun the merge command.

Details for Dev Infra team

Raised by workflow job

ezyang · 2023-02-16T14:07:39Z

@pytorchbot merge

pytorchmergebot · 2023-02-16T14:09:51Z

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

Since I didn't want to deal with nondeterministic tests, I went the exhaustive testing route for a fixed list of constants to look at. The tests generate random ranges, propagate the range through the function, and then pick elements in the range and check that the result on the operation is in the resulting range. This caught bugs in log, sqrt and pow. My resolution for pow was a little special, because I had trouble figuring out the correct semantics under all inputs domains. Instead, I picked two input domains (pow on two point ranges, and pow where exponent is known) and only implemented those. Everything else we give up. I think this is unlikely to affect perf. Signed-off-by: Edward Z. Yang <ezyangmeta.com> [ghstack-poisoned]

pytorchmergebot · 2023-02-16T14:25:04Z

Merge failed

Reason: New commits were pushed while merging. Please rerun the merge command.

Details for Dev Infra team

Raised by workflow job

nunoplopes

LGTM!

Since I didn't want to deal with nondeterministic tests, I went the exhaustive testing route for a fixed list of constants to look at. The tests generate random ranges, propagate the range through the function, and then pick elements in the range and check that the result on the operation is in the resulting range. This caught bugs in log, sqrt and pow. My resolution for pow was a little special, because I had trouble figuring out the correct semantics under all inputs domains. Instead, I picked two input domains (pow on two point ranges, and pow where exponent is known) and only implemented those. Everything else we give up. I think this is unlikely to affect perf. Signed-off-by: Edward Z. Yang <ezyangmeta.com> cc gujinghui PenghuiCheng XiaobingSuper jianyuh jgong5 mingfeima sanchitintel ashokei jingxu10 min-jean-cho yanbing-j Guobing-Chen Xia-Weiwen [ghstack-poisoned]

yanbing-j · 2023-02-28T02:28:49Z

Hi @ezyang , could you please double check the code change of third_party/ideep? I suppose the change is unnecessary, could you please reland this PR then? Thanks!

ezyang · 2023-02-28T03:05:40Z

It's not intentional. You won't be able to revert this PR, please submit a forward fix PR.

yanbing-j · 2023-02-28T05:18:43Z

Please review the forward fix PR #95688. Thanks!

### Description This PR is to update ideep to add primitive cache in order to speed up ARM's PyTorch workloads. Reland #94719, which is unintentional reverted by #94939 (comment). Fixes #94264. ### Performance test Use TorchBench test in ICX with 40 cores Intel OpenMP & jemalloc were preloaded ![image](https://user-images.githubusercontent.com/61222868/221760391-fb6cbabe-6d88-4155-b216-348e718e68b9.png) Pull Request resolved: #95688 Approved by: https://github.com/ezyang

### Description This PR is to update ideep to add primitive cache in order to speed up ARM's PyTorch workloads. Reland pytorch/pytorch#94719, which is unintentional reverted by pytorch/pytorch#94939 (comment). Fixes pytorch/pytorch#94264. ### Performance test Use TorchBench test in ICX with 40 cores Intel OpenMP & jemalloc were preloaded ![image](https://user-images.githubusercontent.com/61222868/221760391-fb6cbabe-6d88-4155-b216-348e718e68b9.png) Pull Request resolved: pytorch/pytorch#95688 Approved by: https://github.com/ezyang

This reverts commit 08ef83f.

Since I didn't want to deal with nondeterministic tests, I went the exhaustive testing route for a fixed list of constants to look at. The tests generate random ranges, propagate the range through the function, and then pick elements in the range and check that the result on the operation is in the resulting range. This caught bugs in log, sqrt and pow. My resolution for pow was a little special, because I had trouble figuring out the correct semantics under all inputs domains. Instead, I picked two input domains (pow on two point ranges, and pow where exponent is known) and only implemented those. Everything else we give up. I think this is unlikely to affect perf. Signed-off-by: Edward Z. Yang <ezyang@meta.com> Pull Request resolved: pytorch#94939 Approved by: https://github.com/lezcano, https://github.com/eellison, https://github.com/nunoplopes

Add fuzz testing to ValueRanges, fix bugs

4f653ec

Signed-off-by: Edward Z. Yang <ezyang@meta.com> [ghstack-poisoned]

ezyang mentioned this pull request Feb 15, 2023

Add some simple sanity tests to ValueRanges #94905

Closed

ezyang mentioned this pull request Feb 15, 2023

Assert more invariants on ValueRanges #94906

Closed

ezyang added a commit that referenced this pull request Feb 15, 2023

Add fuzz testing to ValueRanges, fix bugs

df5a418

Signed-off-by: Edward Z. Yang <ezyangmeta.com> ghstack-source-id: 6a3d0236bcf4ad1e8b8070a519c2f802d9105c62 Pull Request resolved: #94939

github-actions bot requested review from albanD, antoniojkim, bdhirsh, Chillee, jbschlosser, miladm, SherlockNoMad, voznesenskym and wconstab February 15, 2023 21:47

albanD removed their request for review February 15, 2023 21:48

ezyang requested review from lezcano, nunoplopes and eellison February 15, 2023 22:20

ezyang changed the title ~~Add fuzz testing to ValueRanges, fix bugs~~ Add exhaustive testing to ValueRanges, fix bugs Feb 15, 2023

ezyang added topic: bug fixes topic category release notes: dynamo release notes: inductor and removed release notes: dynamo labels Feb 15, 2023

lezcano approved these changes Feb 15, 2023

View reviewed changes

ezyang mentioned this pull request Feb 15, 2023

Add boolean/comparison operator support to ValueRanges #94944

Closed

eellison approved these changes Feb 16, 2023

View reviewed changes

pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Feb 16, 2023

github-actions bot added the module: mkldnn Related to Intel IDEEP or oneDNN (a.k.a. mkldnn) integration label Feb 16, 2023

nunoplopes approved these changes Feb 16, 2023

View reviewed changes

ezyang mentioned this pull request Feb 16, 2023

Add torch.utils._sympy.interp #94985

Closed

ezyang mentioned this pull request Feb 17, 2023

Introduce constrain_range; remove old expr_subs #95063

Closed

pytorchmergebot added the Merged label Feb 17, 2023

pytorchmergebot closed this in 08ef83f Feb 17, 2023

yanbing-j mentioned this pull request Feb 28, 2023

Reland #94719 - Update ideep to add primitive cache for ARM #95688

Closed

msaroufim mentioned this pull request Mar 3, 2023

Remove mention of dynamo.optimize() in docs #96002

Closed

pruthvistony added a commit to ROCm/pytorch that referenced this pull request May 2, 2023

Revert "Add exhaustive testing to ValueRanges, fix bugs (pytorch#94939)"

dbaecf8

This reverts commit 08ef83f.

facebook-github-bot deleted the gh/ezyang/1819/head branch June 8, 2023 16:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add exhaustive testing to ValueRanges, fix bugs #94939

Add exhaustive testing to ValueRanges, fix bugs #94939

ezyang commented Feb 15, 2023 •

edited

Loading

pytorch-bot bot commented Feb 15, 2023 •

edited

Loading

lezcano Feb 15, 2023

lezcano Feb 15, 2023

ezyang Feb 15, 2023

lezcano Feb 16, 2023

eellison left a comment

eellison Feb 16, 2023

ezyang Feb 16, 2023

ezyang commented Feb 16, 2023

pytorchmergebot commented Feb 16, 2023

pytorchmergebot commented Feb 16, 2023

ezyang commented Feb 16, 2023

pytorchmergebot commented Feb 16, 2023

pytorchmergebot commented Feb 16, 2023

nunoplopes left a comment

yanbing-j commented Feb 28, 2023

ezyang commented Feb 28, 2023

yanbing-j commented Feb 28, 2023

		# This is fairly difficult to analyze, so give up for anything
		# complicated

		@@ -119,15 +122,45 @@ def ceil(x):
		return math.ceil(x)


		def valid_unary(fn, v):

Add exhaustive testing to ValueRanges, fix bugs #94939

Add exhaustive testing to ValueRanges, fix bugs #94939

Conversation

ezyang commented Feb 15, 2023 • edited Loading

pytorch-bot bot commented Feb 15, 2023 • edited Loading

🔗 Helpful Links

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

✅ No Failures

lezcano Feb 15, 2023

Choose a reason for hiding this comment

lezcano Feb 15, 2023

Choose a reason for hiding this comment

ezyang Feb 15, 2023

Choose a reason for hiding this comment

lezcano Feb 16, 2023

Choose a reason for hiding this comment

eellison left a comment

Choose a reason for hiding this comment

eellison Feb 16, 2023

Choose a reason for hiding this comment

ezyang Feb 16, 2023

Choose a reason for hiding this comment

ezyang commented Feb 16, 2023

pytorchmergebot commented Feb 16, 2023

Merge started

pytorchmergebot commented Feb 16, 2023

Merge failed

ezyang commented Feb 16, 2023

pytorchmergebot commented Feb 16, 2023

Merge started

pytorchmergebot commented Feb 16, 2023

Merge failed

nunoplopes left a comment

Choose a reason for hiding this comment

yanbing-j commented Feb 28, 2023

ezyang commented Feb 28, 2023

yanbing-j commented Feb 28, 2023

ezyang commented Feb 15, 2023 •

edited

Loading

pytorch-bot bot commented Feb 15, 2023 •

edited

Loading