Stops common_utils.py from setting the default tensor type (to torch.DoubleTensor) #27444

mruberry · 2019-10-06T13:24:19Z

This PR stop common_utils.py from setting the default tensor type when it's imported. See issue #27355. This is a frequent source of confusion for test writers.

Many tests relied on this setting (whether they knew it or not), and this PR also updates the test suite to pass without common_utils.py setting the default tensor type. Some larger test files now set the default floating dtype themselves, however. These test files are:

test_autograd.py
test_distributions.py
test_jit.py
test_nn.py

This is still a significant improvement from today, however. First, these files set the default floating dtype much more clearly than importing it from common_utils. Second, the rest of the test suite no longer sets this globally. Third, this PR is a springboard to updating those tests, too. In particular, as tests are made generic they can be moved aways from relying on this global setting.

Notable technical changes in this PR are:

Significant updates to test_torch.py to make it pass without setting the default floating dtype globally.
The @default_floating_dtype decorator is now defined in common_utils, a couple versions of this operator were defined in test files previously.
test_torch-specific parts of common_utils were refactored into test_torch.
tensor creation methods in common_utils were updated to accept an optional dtype and device.

…ailures

mruberry · 2019-10-07T03:59:55Z

The ASAN build failures are unrelated but mandate a retest. Will wait until review in case there are changes (which will the retrigger the CI).

ezyang · 2019-10-07T14:12:04Z

How are you "testing" this change? :)

ezyang · 2019-10-07T14:18:50Z

test/common_utils.py

    return u.matmul(s.expand(batch_dims + (matrix_size, matrix_size)).matmul(v.transpose(-2, -1)))


-def lu_solve_test_helper(self, A_dims, b_dims, cast, pivot):


Heh, dead code, nice!

This code was inlined into test_torch, its only caller.

ezyang · 2019-10-07T14:21:23Z

test/test_autograd.py

+
+# TODO: remove this global setting
+# Autograd tests use double as the default dtype
+torch.set_default_dtype(torch.double)


So, I recall that you noticed that the cuda autograd tests were done with float. Is that still the case after this change?

I must have been mistaken. Previously an equivalent import came from common_utils, and this setting also applies to CUDA tensors. There is no functional change here.

ezyang

In the interest of avoiding merge conflicts I'm approving this after only a cursory review. I am still curious about how the changes are bbeing "validated" though.

mruberry · 2019-10-07T17:07:39Z

"Validation" is an interesting question.

We have changed the way tests work in a majority of the Python test suite to use float instead of double for floating point tensors.

Tests that can pass with this change are left with it. This is a change to our testing surface (we are testing double tensors much less). Tests that require the default floating dtype to be double maintain the previous behavior. I was conservative in keeping tests in double to avoid this PR being too large, hence the four test files that declare the default dtype to be double immediately.

Aside from the test suite ensuring this change does not break our CI, I also plan to do a follow-up review on test coverage (which will include dtype coverage). Another issue with this plan is that third-parties may have relied (knowingly or otherwise) on common_utils setting the default tensor dtype. However I think it would be productive to break their test suites and make them explicitly declare the behavior if they like, or start ensuring their float behavior works as expected.

mruberry · 2019-10-07T17:08:00Z

@pytorchbot rebase this please.

mruberry · 2019-10-07T17:13:56Z

Rebase is to validate ASAN builds pass test CI. They failed to build in the previous CI run.

facebook-github-bot

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

into default_test_tensor

facebook-github-bot

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

facebook-github-bot · 2019-10-08T18:09:37Z

@mruberry merged this pull request in 7f183a9.

…DoubleTensor) (pytorch#27444) Summary: This PR stop common_utils.py from setting the default tensor type when it's imported. See issue pytorch#27355. This is a frequent source of confusion for test writers. Many tests relied on this setting (whether they knew it or not), and this PR also updates the test suite to pass without common_utils.py setting the default tensor type. Some larger test files now set the default floating dtype themselves, however. These test files are: - test_autograd.py - test_distributions.py - test_jit.py - test_nn.py This is still a significant improvement from today, however. First, these files set the default floating dtype much more clearly than importing it from common_utils. Second, the rest of the test suite no longer sets this globally. Third, this PR is a springboard to updating those tests, too. In particular, as tests are made generic they can be moved aways from relying on this global setting. Notable technical changes in this PR are: - Significant updates to test_torch.py to make it pass without setting the default floating dtype globally. - The default_floating_dtype decorator is now defined in common_utils, a couple versions of this operator were defined in test files previously. - test_torch-specific parts of common_utils were refactored into test_torch. - tensor creation methods in common_utils were updated to accept an optional dtype and device. Pull Request resolved: pytorch#27444 Differential Revision: D17795235 Pulled By: mruberry fbshipit-source-id: 7f77271c0c836e69f183ad9057a2c4b29f09d2e1

Removes common_utils.py set default tensor type

6162ff6

mruberry requested review from mrshenli and pietern as code owners October 6, 2019 13:24

mruberry removed request for pietern and mrshenli October 6, 2019 13:55

Fixes Python2, adds test_dataloader annotations

e2d45c9

pytorchbot added the module: dataloader Related to torch.utils.data.DataLoader and Sampler label Oct 6, 2019

Updates for test_c10d.py, test_cpp_extensions.py

761beaf

pytorchbot added the module: cpp-extensions Related to torch.utils.cpp_extension label Oct 6, 2019

Mike Ruberry added 2 commits October 6, 2019 14:30

Leaves test_jit and test_distributions on double, updates per niche f…

0a1fc43

…ailures

test_stft needs double on some builds

5d00041

mruberry changed the title ~~[WIP - Testing] Removes common_utils.py set default tensor type~~ Stops common_utils.py from setting the default tensor type (to torch.DoubleTensor) Oct 7, 2019

mruberry requested review from dzhulgakov and ezyang October 7, 2019 03:59

mruberry mentioned this pull request Oct 7, 2019

Python tests use torch.Doubletensor as their default tensor type #27355

Closed

ezyang reviewed Oct 7, 2019

View reviewed changes

ezyang approved these changes Oct 7, 2019

View reviewed changes

Merge remote-tracking branch 'origin/master' into HEAD

06a911f

facebook-github-bot reviewed Oct 7, 2019

View reviewed changes

Mike Ruberry added 2 commits October 7, 2019 16:03

puts forks on double

23b220c

Merge branch 'default_test_tensor' of https://github.com/pytorch/pytorch

7ac94de

into default_test_tensor

facebook-github-bot reviewed Oct 7, 2019

View reviewed changes

facebook-github-bot closed this in 7f183a9 Oct 8, 2019

mruberry deleted the default_test_tensor branch October 8, 2019 16:57

facebook-github-bot added the merged label Oct 8, 2019

mruberry mentioned this pull request Jul 16, 2020

Don't use DoubleTensor as default global type in common.py #4588

Closed

mruberry added the Merged label Oct 28, 2020

ejguan mentioned this pull request Nov 4, 2020

[Big question]Why test_nn use double dtype as cpu input and why set dtype2prec_DONTUSE? #47340

Closed

Linux-cpp-lisp mentioned this pull request Feb 18, 2021

Fix support for device and dtype e3nn/e3nn#203

Merged

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Stops common_utils.py from setting the default tensor type (to torch.DoubleTensor) #27444

Stops common_utils.py from setting the default tensor type (to torch.DoubleTensor) #27444

mruberry commented Oct 6, 2019 •

edited

mruberry commented Oct 7, 2019

ezyang commented Oct 7, 2019

ezyang Oct 7, 2019

mruberry Oct 7, 2019

ezyang Oct 7, 2019

mruberry Oct 7, 2019

ezyang left a comment

mruberry commented Oct 7, 2019

mruberry commented Oct 7, 2019

mruberry commented Oct 7, 2019

facebook-github-bot left a comment

facebook-github-bot left a comment

facebook-github-bot commented Oct 8, 2019

		return u.matmul(s.expand(batch_dims + (matrix_size, matrix_size)).matmul(v.transpose(-2, -1)))


		def lu_solve_test_helper(self, A_dims, b_dims, cast, pivot):

Stops common_utils.py from setting the default tensor type (to torch.DoubleTensor) #27444

Stops common_utils.py from setting the default tensor type (to torch.DoubleTensor) #27444

Conversation

mruberry commented Oct 6, 2019 • edited

mruberry commented Oct 7, 2019

ezyang commented Oct 7, 2019

ezyang Oct 7, 2019

Choose a reason for hiding this comment

mruberry Oct 7, 2019

Choose a reason for hiding this comment

ezyang Oct 7, 2019

Choose a reason for hiding this comment

mruberry Oct 7, 2019

Choose a reason for hiding this comment

ezyang left a comment

Choose a reason for hiding this comment

mruberry commented Oct 7, 2019

mruberry commented Oct 7, 2019

mruberry commented Oct 7, 2019

facebook-github-bot left a comment

Choose a reason for hiding this comment

facebook-github-bot left a comment

Choose a reason for hiding this comment

facebook-github-bot commented Oct 8, 2019

mruberry commented Oct 6, 2019 •

edited