Add correction parameter to std/var#50903
Closed
peterbell10 wants to merge 43 commits intogh/peterbell10/47/basefrom
Closed
Add correction parameter to std/var#50903peterbell10 wants to merge 43 commits intogh/peterbell10/47/basefrom
peterbell10 wants to merge 43 commits intogh/peterbell10/47/basefrom
Conversation
[ghstack-poisoned]
This was referenced Jan 21, 2021
Contributor
💊 CI failures summary and remediationsAs of commit 94a0fa4 (more details on the Dr. CI page):
🕵️ 3 new failures recognized by patternsThe following CI failures do not appear to be due to upstream breakages:
|
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
First part of #50010 [ghstack-poisoned]
First part of #50010 [ghstack-poisoned]
First part of #50010 [ghstack-poisoned]
First part of #50010 [ghstack-poisoned]
First part of #50010 [ghstack-poisoned]
First part of #50010 [ghstack-poisoned]
Collaborator
|
Looks like a complex test is failing internally: Maybe the test is sensitive to different random values, a different version of NumPy, or a different compiler? We might need to make the test "easier" or simplify it. The complex double version of the test is failing with the same error. |
First part of #50010. Also fixes #51127. Differential Revision: [D27911345](https://our.internmc.facebook.com/intern/diff/D27911345) [ghstack-poisoned]
Collaborator
Author
@mruberry the test is sensitive to machine precision, but only in the event the variance is zero (since exactly zero gives nan, while finite but small gives inf). However, zero variance seems very unlikely on random data so that's still a bit surprising and might be worth looking into. For now, I've changed the test to treat nan and inf as equal. |
Collaborator
|
Unfortunately the same tests are failing: If it's a pain to sort out what's going I could try to get someone with access to the internal test harness to take a look? |
First part of #50010. Also fixes #51127. Differential Revision: [D27911345](https://our.internmc.facebook.com/intern/diff/D27911345) [ghstack-poisoned]
Collaborator
Author
|
@mruberry I've loosened the restrictions on the imaginary component this time. Allowing to to be a non-finite value if the real component also isn't finite. If that doesn't work, I really need to know what arguments to |
Collaborator
Sounds good; let's see what the internal tests say. |
Collaborator
|
Good news! Internal tests are now passing. I'll get the land process started tomorrow morning and ping @JackCaoG with updates |
Collaborator
|
Darn, looks like the PR CI failures for fx are real: test_normalize_operator_exhaustive_std_cpu_float32 Probably just a tweak to that test is needed? |
First part of #50010. Also fixes #51127. Differential Revision: [D27911345](https://our.internmc.facebook.com/intern/diff/D27911345) [ghstack-poisoned]
First part of #50010. Also fixes #51127. Differential Revision: [D27911345](https://our.internmc.facebook.com/intern/diff/D27911345) [ghstack-poisoned]
Collaborator
Author
|
@mruberry since the fx PR got reverted, I've removed it from this stack and instead just added a skip in the test for |
First part of #50010. Also fixes #51127. Differential Revision: [D27911345](https://our.internmc.facebook.com/intern/diff/D27911345) [ghstack-poisoned]
peterbell10
added a commit
to peterbell10/pytorch
that referenced
this pull request
Apr 29, 2021
ghstack-source-id: 49aef26 Pull Request resolved: pytorch#50903
Collaborator
Collaborator
|
We'll have to try again on Monday, sorry @peterbell10. Some landing issues let this accumulate another merge conflict. Let's rebase it Monday morning and I'll start the land process while no one's committing. |
peterbell10
added a commit
to peterbell10/pytorch
that referenced
this pull request
May 4, 2021
ghstack-source-id: 49aef26 Pull Request resolved: pytorch#50903
First part of #50010. Also fixes #51127. Differential Revision: [D27911345](https://our.internmc.facebook.com/intern/diff/D27911345) [ghstack-poisoned]
Collaborator
Author
|
@mruberry I've rebased on viable/strict and fixed merge conflicts. |
Collaborator
|
Internal tests look good; unfortunately I lost power this morning, but I'll try to coordinate this with @JackCaoG tomorrow morning |
Contributor
krshrimali
pushed a commit
to krshrimali/pytorch
that referenced
this pull request
May 19, 2021
Summary: Pull Request resolved: pytorch#50903 First part of pytorch#50010. Also fixes pytorch#51127. Test Plan: Imported from OSS Reviewed By: ngimel Differential Revision: D27911345 Pulled By: mruberry fbshipit-source-id: 7138fddc935802918ab9ff19f4bc1b9f4d745d41
jasperzhong
pushed a commit
to jasperzhong/swift
that referenced
this pull request
Nov 25, 2021
ghstack-source-id: 98ac0e1 Pull Request resolved: pytorch/pytorch#50903
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stack from ghstack:
correctionvalue andunbiasedargument #55679 std/var: Deprecate default correction valueFirst part of #50010. Also fixes #51127.
Added overloads for torch.{std, var, std_mean, var_mean} with a correction argument specifying the difference between the sample size and number of degrees of freedom. e.g
correction=1is equivalent to Bessel's correction, which can also be gained using theunbiased=Trueoverload.Differential Revision: D27911345