Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[CherryPick] Fixes for distribution validation checks #53763

Merged
merged 4 commits into from
Mar 15, 2021
Merged

[CherryPick] Fixes for distribution validation checks #53763

merged 4 commits into from
Mar 15, 2021

Conversation

neerajprad
Copy link
Contributor

@neerajprad neerajprad commented Mar 10, 2021 •
edited
Loading

This has fixes for distribution validation checks from the following PRs in master:

These fixes are required for a consistent behavior given that distribution argument validation is now enabled by default. Together these PRs ensure that all distribution log_prob methods call sample validation when validate_args=True and, conversely, turn off all validation checks when validate_args=False.

neerajprad and others added 4 commits March 10, 2021 14:36
Add sample validation for LKJCholesky.log_prob
d49445c
@t-vi
Fix distributions which don't properly honor validate_args=False
4051b73 
A number of derived distributions use base distributions in their
implementation.

We add what we hope is a comprehensive test whether all distributions
actually honor skipping validation of arguments in log_prob and then
fix the bugs we found. These bugs are particularly cumbersome in
PyTorch 1.8 and master when validate_args is turned on by default
In addition one might argue that validate_args is not performing
as well as it should when the default is not to validate but the
validation is turned on in instantiation.

Arguably, there is another set of bugs or at least inconsistencies
when validation of inputs does not prevent invalid indices in
sample validation (when with validation an IndexError is raised
in the test). We would encourage the implementors to be more
ambitious when validation is turned on and amend sample validation
to throw a ValueError for consistency.
additional fixes to distributions
eddbdf2
address failing tests
b9065bd
@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Mar 10, 2021 •
edited
Loading

💊 CI failures summary and remediations

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

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

🕵️ 1 new failure recognized by patterns

The following CI failures do not appear to be due to upstream breakages:

See CircleCI build pytorch_xla_linux_bionic_py3_6_clang9_test (1/1)

Step: "Run tests" (full log | diagnosis details | 🔁 rerun)

Mar 11 00:03:55 AssertionError: False is not true 
Mar 11 00:03:55   test_where_scalar_valid_combination_xla_uint8 (__main__.TestTorchDeviceTypeXLA) ... ok (0.028s)
Mar 11 00:03:55 
Mar 11 00:03:55 ======================================================================
Mar 11 00:03:55 FAIL [0.002s]: test_pickle_gradscaler_xla (__main__.TestTorchDeviceTypeXLA)
Mar 11 00:03:55 ----------------------------------------------------------------------
Mar 11 00:03:55 Traceback (most recent call last):
Mar 11 00:03:55   File "/opt/conda/lib/python3.6/site-packages/torch/testing/_internal/common_device_type.py", line 290, in instantiated_test
Mar 11 00:03:55     result = test_fn(self, *args)
Mar 11 00:03:55   File "/var/lib/jenkins/workspace/xla/test/../../test/test_torch.py", line 6172, in test_pickle_gradscaler
Mar 11 00:03:55     self.assertTrue(a.is_enabled() if torch.cuda.is_available() else not a.is_enabled())
Mar 11 00:03:55 AssertionError: False is not true
Mar 11 00:03:55 
Mar 11 00:03:56 ----------------------------------------------------------------------
Mar 11 00:03:56 Ran 244 tests in 149.565s
Mar 11 00:03:56 
Mar 11 00:03:56 FAILED (failures=1, skipped=140)
Mar 11 00:03:56 
Mar 11 00:03:56 Generating XML reports...
Mar 11 00:03:56 Generated XML report: test-reports/python-unittest/TEST-TestTorchDeviceTypeXLA-20210311000126.xml
Mar 11 00:03:56 + cleanup
Mar 11 00:03:56 + retcode=1

ci.pytorch.org: 1 failed

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.

@neerajprad neerajprad mentioned this pull request Mar 11, 2021
Closed
@malfet malfet merged commit e991cda into pytorch:release/1.8 Mar 15, 2021
@tingiskhan tingiskhan mentioned this pull request Apr 3, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@malfet malfet malfet approved these changes

Assignees
No one assigned
Labels
cla signed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@neerajprad @facebook-github-bot @malfet @t-vi