Skip to content

remove randn? from torch.testing namespace #61840

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.

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

Closed
wants to merge 3 commits into from

Conversation

pmeier
Copy link
Collaborator

@pmeier pmeier commented Jul 19, 2021

Stack from ghstack:

Redo of #60859.

Differential Revision: D29871017

@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Jul 19, 2021

💊 CI failures summary and remediations

As of commit 37bbf88 (more details on the Dr. CI page and at hud.pytorch.org/pr/61840):


💚 💚 Looks good so far! There are no failures yet. 💚 💚


Preview docs built from this PR

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.

Click here to manually regenerate this comment.

@mruberry
Copy link
Collaborator

There is only one appearance of "from torch.testing import randn_like" in test_autograd.py and no appearances of "from torch.testing import rand_like" within Facebook.

"torch.testing.randn_like" appears in test_autograd.py and gradcheck.py, while "torch.testing.rand_like" never appears.

In addition to these, "make_non_contiguous" is imported by only one project outside PyTorch. It is used in test_torch.py and gradcheck.py, though, so maybe that's a good candidate for a follow-up PR?

"FileCheck" is used by several projects within Facebook, so that will require a focused follow-up.

"assert_allclose" is used by several projects OUTSIDE of Facebook, so that will require a proper deprecation (see comments on that PR -- seems like that deprecation should wait for a stable API to use as an alternative?)

The remaining functions exposed here:

__all__ = [

are all dtype getters, I think. Some of these dtype getters are used by external projects. "floating_types" and "integral_types" are both used by torchvision, for example, and "get_all_device_types" and "get_all_math_dtypes" are used by additional projects outside PyTorch. I think, however, that we can move all these to deprecated if someone tries to import them from testing and create internal versions of these functions. For torchvision we can just copy the functions they're using, and the few external projects can update themselves with a simple list of dtypes. Updating our internal test suite to use the internal versions of these operations will take some doing, however. A lot of our test suites are calling torch.testing.get_all_device_types(), for example.

How does that sound, @pmeier?

@pmeier
Copy link
Collaborator Author

pmeier commented Jul 21, 2021

There is only one appearance of "from torch.testing import randn_like" in test_autograd.py and no appearances of "from torch.testing import rand_like" within Facebook.

"torch.testing.randn_like" appears in test_autograd.py and gradcheck.py, while "torch.testing.rand_like" never appears.

Seems like this PR is good to go then.

In addition to these, "make_non_contiguous" is imported by only one project outside PyTorch. It is used in test_torch.py and gradcheck.py, though, so maybe that's a good candidate for a follow-up PR?

Yes, I'll do that one next in the stack. The usage in gradcheck will be a problem though. In general, we can't import anything from torch.testing._internal into the "public" part. We could leave it in torch/testing/_core.py and just prefix an underscore to "remove" it. Thoughts?

"FileCheck" is used by several projects within Facebook, so that will require a focused follow-up.

Sounds good.

"assert_allclose" is used by several projects OUTSIDE of Facebook, so that will require a proper deprecation (see comments on that PR -- seems like that deprecation should wait for a stable API to use as an alternative?)

For reference: #61844.

I'm against waiting that long for a deprecation for two reasons:

  1. It can easily be confused with assert_close in the same namespace. Since assert_close will be the successor I think we should avoid that.
  2. I would relax the "only deprecate in favor of stable" towards "only deprecate in favor of something more stable". If we assume stable > beta > prototype > undocumented we would be good to deprecate right away.

The remaining functions exposed here:

__all__ = [

are all dtype getters, I think. Some of these dtype getters are used by external projects. "floating_types" and "integral_types" are both used by torchvision, for example, and "get_all_device_types" and "get_all_math_dtypes" are used by additional projects outside PyTorch. I think, however, that we can move all these to deprecated if someone tries to import them from testing and create internal versions of these functions. For torchvision we can just copy the functions they're using, and the few external projects can update themselves with a simple list of dtypes.

Agreed. The largest problem that I see is that the naming scheme is very confusing if one is not aware of the organic growth of dtypes over time. For example, I bet other libraries using get_all_math_dtypes do not know that they are not in fact getting all math dtypes:

def get_all_math_dtypes(device) -> List[torch.dtype]:
return get_all_int_dtypes() + get_all_fp_dtypes(include_half=device.startswith('cuda'),
include_bfloat16=False) + get_all_complex_dtypes()

Updating our internal test suite to use the internal versions of these operations will take some doing, however. A lot of our test suites are calling torch.testing.get_all_device_types(), for example.

Should be a fairly easy substitution. Might be some busy work, but I don't expect any larger obstacles.

pmeier added a commit to pmeier/pytorch that referenced this pull request Jul 21, 2021
Redo of pytorch#60859.

ghstack-source-id: 9ccf58d
Pull Request resolved: pytorch#61840
Copy link
Collaborator

@mruberry mruberry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool approach to deprecations

@mruberry
Copy link
Collaborator

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

@facebook-github-bot
Copy link
Contributor

@mruberry merged this pull request in 10ccc5a.

@facebook-github-bot facebook-github-bot deleted the gh/pmeier/30/head branch July 27, 2021 14:17
pmeier added a commit that referenced this pull request Aug 19, 2021
Following #61840 (comment), this deprecates all the dtype getters publicly exposed in the `torch.testing` namespace. The reason for this twofold:

1. If someone is not familiar with the C++ dispatch macros PyTorch uses, the names are misleading. For example `torch.testing.floating_types()` will only give you `float32` and `float64` skipping `float16` and `bfloat16`.
2. The dtype getters provide very minimal functionality that can be easily emulated by downstream libraries.

We thought about [providing an replacement](https://gist.github.com/pmeier/3dfd2e105842ad0de4505068a1a0270a), but ultimately decided against it. The major problem is BC: by keeping it, either the namespace is getting messy again after a new dtype is added or we need to somehow version the return values of the getters.

[ghstack-poisoned]
pmeier added a commit that referenced this pull request Aug 19, 2021
Following #61840 (comment), this deprecates all the dtype getters publicly exposed in the `torch.testing` namespace. The reason for this twofold:

1. If someone is not familiar with the C++ dispatch macros PyTorch uses, the names are misleading. For example `torch.testing.floating_types()` will only give you `float32` and `float64` skipping `float16` and `bfloat16`.
2. The dtype getters provide very minimal functionality that can be easily emulated by downstream libraries.

We thought about [providing an replacement](https://gist.github.com/pmeier/3dfd2e105842ad0de4505068a1a0270a), but ultimately decided against it. The major problem is BC: by keeping it, either the namespace is getting messy again after a new dtype is added or we need to somehow version the return values of the getters.

[ghstack-poisoned]
pmeier added a commit that referenced this pull request Aug 24, 2021
…g` namespace"


Following #61840 (comment), this deprecates all the dtype getters publicly exposed in the `torch.testing` namespace. The reason for this twofold:

1. If someone is not familiar with the C++ dispatch macros PyTorch uses, the names are misleading. For example `torch.testing.floating_types()` will only give you `float32` and `float64` skipping `float16` and `bfloat16`.
2. The dtype getters provide very minimal functionality that can be easily emulated by downstream libraries.

We thought about [providing an replacement](https://gist.github.com/pmeier/3dfd2e105842ad0de4505068a1a0270a), but ultimately decided against it. The major problem is BC: by keeping it, either the namespace is getting messy again after a new dtype is added or we need to somehow version the return values of the getters.

[ghstack-poisoned]
pmeier added a commit that referenced this pull request Aug 24, 2021
Following #61840 (comment), this deprecates all the dtype getters publicly exposed in the `torch.testing` namespace. The reason for this twofold:

1. If someone is not familiar with the C++ dispatch macros PyTorch uses, the names are misleading. For example `torch.testing.floating_types()` will only give you `float32` and `float64` skipping `float16` and `bfloat16`.
2. The dtype getters provide very minimal functionality that can be easily emulated by downstream libraries.

We thought about [providing an replacement](https://gist.github.com/pmeier/3dfd2e105842ad0de4505068a1a0270a), but ultimately decided against it. The major problem is BC: by keeping it, either the namespace is getting messy again after a new dtype is added or we need to somehow version the return values of the getters.

[ghstack-poisoned]
pmeier added a commit that referenced this pull request Aug 26, 2021
…g` namespace"


Following #61840 (comment), this deprecates all the dtype getters publicly exposed in the `torch.testing` namespace. The reason for this twofold:

1. If someone is not familiar with the C++ dispatch macros PyTorch uses, the names are misleading. For example `torch.testing.floating_types()` will only give you `float32` and `float64` skipping `float16` and `bfloat16`.
2. The dtype getters provide very minimal functionality that can be easily emulated by downstream libraries.

We thought about [providing an replacement](https://gist.github.com/pmeier/3dfd2e105842ad0de4505068a1a0270a), but ultimately decided against it. The major problem is BC: by keeping it, either the namespace is getting messy again after a new dtype is added or we need to somehow version the return values of the getters.

[ghstack-poisoned]
pmeier added a commit that referenced this pull request Aug 26, 2021
Following #61840 (comment), this deprecates all the dtype getters publicly exposed in the `torch.testing` namespace. The reason for this twofold:

1. If someone is not familiar with the C++ dispatch macros PyTorch uses, the names are misleading. For example `torch.testing.floating_types()` will only give you `float32` and `float64` skipping `float16` and `bfloat16`.
2. The dtype getters provide very minimal functionality that can be easily emulated by downstream libraries.

We thought about [providing an replacement](https://gist.github.com/pmeier/3dfd2e105842ad0de4505068a1a0270a), but ultimately decided against it. The major problem is BC: by keeping it, either the namespace is getting messy again after a new dtype is added or we need to somehow version the return values of the getters.

[ghstack-poisoned]
pmeier added a commit that referenced this pull request Aug 26, 2021
…g` namespace"


Following #61840 (comment), this deprecates all the dtype getters publicly exposed in the `torch.testing` namespace. The reason for this twofold:

1. If someone is not familiar with the C++ dispatch macros PyTorch uses, the names are misleading. For example `torch.testing.floating_types()` will only give you `float32` and `float64` skipping `float16` and `bfloat16`.
2. The dtype getters provide very minimal functionality that can be easily emulated by downstream libraries.

We thought about [providing an replacement](https://gist.github.com/pmeier/3dfd2e105842ad0de4505068a1a0270a), but ultimately decided against it. The major problem is BC: by keeping it, either the namespace is getting messy again after a new dtype is added or we need to somehow version the return values of the getters.

[ghstack-poisoned]
pmeier added a commit that referenced this pull request Aug 26, 2021
Following #61840 (comment), this deprecates all the dtype getters publicly exposed in the `torch.testing` namespace. The reason for this twofold:

1. If someone is not familiar with the C++ dispatch macros PyTorch uses, the names are misleading. For example `torch.testing.floating_types()` will only give you `float32` and `float64` skipping `float16` and `bfloat16`.
2. The dtype getters provide very minimal functionality that can be easily emulated by downstream libraries.

We thought about [providing an replacement](https://gist.github.com/pmeier/3dfd2e105842ad0de4505068a1a0270a), but ultimately decided against it. The major problem is BC: by keeping it, either the namespace is getting messy again after a new dtype is added or we need to somehow version the return values of the getters.

[ghstack-poisoned]
pmeier added a commit that referenced this pull request Aug 26, 2021
…g` namespace"


Following #61840 (comment), this deprecates all the dtype getters publicly exposed in the `torch.testing` namespace. The reason for this twofold:

1. If someone is not familiar with the C++ dispatch macros PyTorch uses, the names are misleading. For example `torch.testing.floating_types()` will only give you `float32` and `float64` skipping `float16` and `bfloat16`.
2. The dtype getters provide very minimal functionality that can be easily emulated by downstream libraries.

We thought about [providing an replacement](https://gist.github.com/pmeier/3dfd2e105842ad0de4505068a1a0270a), but ultimately decided against it. The major problem is BC: by keeping it, either the namespace is getting messy again after a new dtype is added or we need to somehow version the return values of the getters.

[ghstack-poisoned]
pmeier added a commit that referenced this pull request Aug 26, 2021
Following #61840 (comment), this deprecates all the dtype getters publicly exposed in the `torch.testing` namespace. The reason for this twofold:

1. If someone is not familiar with the C++ dispatch macros PyTorch uses, the names are misleading. For example `torch.testing.floating_types()` will only give you `float32` and `float64` skipping `float16` and `bfloat16`.
2. The dtype getters provide very minimal functionality that can be easily emulated by downstream libraries.

We thought about [providing an replacement](https://gist.github.com/pmeier/3dfd2e105842ad0de4505068a1a0270a), but ultimately decided against it. The major problem is BC: by keeping it, either the namespace is getting messy again after a new dtype is added or we need to somehow version the return values of the getters.

[ghstack-poisoned]
pmeier added a commit that referenced this pull request Aug 26, 2021
…g` namespace"


Following #61840 (comment), this deprecates all the dtype getters publicly exposed in the `torch.testing` namespace. The reason for this twofold:

1. If someone is not familiar with the C++ dispatch macros PyTorch uses, the names are misleading. For example `torch.testing.floating_types()` will only give you `float32` and `float64` skipping `float16` and `bfloat16`.
2. The dtype getters provide very minimal functionality that can be easily emulated by downstream libraries.

We thought about [providing an replacement](https://gist.github.com/pmeier/3dfd2e105842ad0de4505068a1a0270a), but ultimately decided against it. The major problem is BC: by keeping it, either the namespace is getting messy again after a new dtype is added or we need to somehow version the return values of the getters.

[ghstack-poisoned]
pmeier added a commit that referenced this pull request Aug 26, 2021
Following #61840 (comment), this deprecates all the dtype getters publicly exposed in the `torch.testing` namespace. The reason for this twofold:

1. If someone is not familiar with the C++ dispatch macros PyTorch uses, the names are misleading. For example `torch.testing.floating_types()` will only give you `float32` and `float64` skipping `float16` and `bfloat16`.
2. The dtype getters provide very minimal functionality that can be easily emulated by downstream libraries.

We thought about [providing an replacement](https://gist.github.com/pmeier/3dfd2e105842ad0de4505068a1a0270a), but ultimately decided against it. The major problem is BC: by keeping it, either the namespace is getting messy again after a new dtype is added or we need to somehow version the return values of the getters.

[ghstack-poisoned]
pmeier added a commit that referenced this pull request Sep 6, 2021
…g` namespace"


Following #61840 (comment), this deprecates all the dtype getters publicly exposed in the `torch.testing` namespace. The reason for this twofold:

1. If someone is not familiar with the C++ dispatch macros PyTorch uses, the names are misleading. For example `torch.testing.floating_types()` will only give you `float32` and `float64` skipping `float16` and `bfloat16`.
2. The dtype getters provide very minimal functionality that can be easily emulated by downstream libraries.

We thought about [providing an replacement](https://gist.github.com/pmeier/3dfd2e105842ad0de4505068a1a0270a), but ultimately decided against it. The major problem is BC: by keeping it, either the namespace is getting messy again after a new dtype is added or we need to somehow version the return values of the getters.

Differential Revision: [D30662206](https://our.internmc.facebook.com/intern/diff/D30662206)

[ghstack-poisoned]
pmeier added a commit that referenced this pull request Sep 6, 2021
Following #61840 (comment), this deprecates all the dtype getters publicly exposed in the `torch.testing` namespace. The reason for this twofold:

1. If someone is not familiar with the C++ dispatch macros PyTorch uses, the names are misleading. For example `torch.testing.floating_types()` will only give you `float32` and `float64` skipping `float16` and `bfloat16`.
2. The dtype getters provide very minimal functionality that can be easily emulated by downstream libraries.

We thought about [providing an replacement](https://gist.github.com/pmeier/3dfd2e105842ad0de4505068a1a0270a), but ultimately decided against it. The major problem is BC: by keeping it, either the namespace is getting messy again after a new dtype is added or we need to somehow version the return values of the getters.

Differential Revision: [D30662206](https://our.internmc.facebook.com/intern/diff/D30662206)

[ghstack-poisoned]
pmeier added a commit that referenced this pull request Sep 6, 2021
…g` namespace"


Following #61840 (comment), this deprecates all the dtype getters publicly exposed in the `torch.testing` namespace. The reason for this twofold:

1. If someone is not familiar with the C++ dispatch macros PyTorch uses, the names are misleading. For example `torch.testing.floating_types()` will only give you `float32` and `float64` skipping `float16` and `bfloat16`.
2. The dtype getters provide very minimal functionality that can be easily emulated by downstream libraries.

We thought about [providing an replacement](https://gist.github.com/pmeier/3dfd2e105842ad0de4505068a1a0270a), but ultimately decided against it. The major problem is BC: by keeping it, either the namespace is getting messy again after a new dtype is added or we need to somehow version the return values of the getters.

Differential Revision: [D30662206](https://our.internmc.facebook.com/intern/diff/D30662206)

[ghstack-poisoned]
pmeier added a commit that referenced this pull request Sep 6, 2021
Following #61840 (comment), this deprecates all the dtype getters publicly exposed in the `torch.testing` namespace. The reason for this twofold:

1. If someone is not familiar with the C++ dispatch macros PyTorch uses, the names are misleading. For example `torch.testing.floating_types()` will only give you `float32` and `float64` skipping `float16` and `bfloat16`.
2. The dtype getters provide very minimal functionality that can be easily emulated by downstream libraries.

We thought about [providing an replacement](https://gist.github.com/pmeier/3dfd2e105842ad0de4505068a1a0270a), but ultimately decided against it. The major problem is BC: by keeping it, either the namespace is getting messy again after a new dtype is added or we need to somehow version the return values of the getters.

Differential Revision: [D30662206](https://our.internmc.facebook.com/intern/diff/D30662206)

[ghstack-poisoned]
pmeier added a commit that referenced this pull request Sep 6, 2021
…g` namespace"


Following #61840 (comment), this deprecates all the dtype getters publicly exposed in the `torch.testing` namespace. The reason for this twofold:

1. If someone is not familiar with the C++ dispatch macros PyTorch uses, the names are misleading. For example `torch.testing.floating_types()` will only give you `float32` and `float64` skipping `float16` and `bfloat16`.
2. The dtype getters provide very minimal functionality that can be easily emulated by downstream libraries.

We thought about [providing an replacement](https://gist.github.com/pmeier/3dfd2e105842ad0de4505068a1a0270a), but ultimately decided against it. The major problem is BC: by keeping it, either the namespace is getting messy again after a new dtype is added or we need to somehow version the return values of the getters.

Differential Revision: [D30662206](https://our.internmc.facebook.com/intern/diff/D30662206)

[ghstack-poisoned]
pmeier added a commit that referenced this pull request Sep 6, 2021
Following #61840 (comment), this deprecates all the dtype getters publicly exposed in the `torch.testing` namespace. The reason for this twofold:

1. If someone is not familiar with the C++ dispatch macros PyTorch uses, the names are misleading. For example `torch.testing.floating_types()` will only give you `float32` and `float64` skipping `float16` and `bfloat16`.
2. The dtype getters provide very minimal functionality that can be easily emulated by downstream libraries.

We thought about [providing an replacement](https://gist.github.com/pmeier/3dfd2e105842ad0de4505068a1a0270a), but ultimately decided against it. The major problem is BC: by keeping it, either the namespace is getting messy again after a new dtype is added or we need to somehow version the return values of the getters.

Differential Revision: [D30662206](https://our.internmc.facebook.com/intern/diff/D30662206)

[ghstack-poisoned]
facebook-github-bot pushed a commit that referenced this pull request Sep 7, 2021
Summary:
Pull Request resolved: #63554

Following #61840 (comment), this deprecates all the dtype getters publicly exposed in the `torch.testing` namespace. The reason for this twofold:

1. If someone is not familiar with the C++ dispatch macros PyTorch uses, the names are misleading. For example `torch.testing.floating_types()` will only give you `float32` and `float64` skipping `float16` and `bfloat16`.
2. The dtype getters provide very minimal functionality that can be easily emulated by downstream libraries.

We thought about [providing an replacement](https://gist.github.com/pmeier/3dfd2e105842ad0de4505068a1a0270a), but ultimately decided against it. The major problem is BC: by keeping it, either the namespace is getting messy again after a new dtype is added or we need to somehow version the return values of the getters.

Test Plan: Imported from OSS

Reviewed By: H-Huang

Differential Revision: D30662206

Pulled By: mruberry

fbshipit-source-id: a2bdb10ab02ae665df1b5b76e8afa9af043bbf56
jasperzhong pushed a commit to jasperzhong/swift that referenced this pull request Nov 25, 2021
Redo of #60859.

ghstack-source-id: dad2fc7
Pull Request resolved: pytorch/pytorch#61840
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants