Skip to content

Rename set_deterministic to use_deterministic_algorithms#49904

Closed
kurtamohler wants to merge 1 commit intopytorch:masterfrom
kurtamohler:use_deterministic_algorithms
Closed

Rename set_deterministic to use_deterministic_algorithms#49904
kurtamohler wants to merge 1 commit intopytorch:masterfrom
kurtamohler:use_deterministic_algorithms

Conversation

@kurtamohler
Copy link
Copy Markdown
Collaborator

Fixes #49100

@facebook-github-bot
Copy link
Copy Markdown
Contributor

facebook-github-bot commented Dec 28, 2020

💊 CI failures summary and remediations

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



❄️ 1 failure tentatively classified as flaky

but reruns have not yet been triggered to confirm:

See CircleCI build pytorch_linux_xenial_cuda10_2_cudnn7_py3_gcc7_test2 (1/1)

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

Jan 14 05:43:21 RuntimeError: CUDA error: an illegal memory access was encountered
Jan 14 05:43:21                        ~~~~ <--- HERE
Jan 14 05:43:21 RuntimeError: CUDA error: an illegal memory access was encountered
Jan 14 05:43:21 
Jan 14 05:43:21 
Jan 14 05:43:21 ======================================================================
Jan 14 05:43:21 ERROR [0.203s]: test_where_and_typing (__main__.TestTEFuser)
Jan 14 05:43:21 ----------------------------------------------------------------------
Jan 14 05:43:21 Traceback (most recent call last):
Jan 14 05:43:21   File "test_jit_fuser_te.py", line 1142, in test_where_and_typing
Jan 14 05:43:21     x = torch.randn(4, 4, dtype=torch.double, device=device)
Jan 14 05:43:21 RuntimeError: CUDA error: an illegal memory access was encountered
Jan 14 05:43:21 
Jan 14 05:43:21 ======================================================================
Jan 14 05:43:21 ERROR [0.176s]: test_zero_element_tensors_cuda (__main__.TestTEFuser)
Jan 14 05:43:21 ----------------------------------------------------------------------
Jan 14 05:43:21 Traceback (most recent call last):
Jan 14 05:43:21   File "/opt/conda/lib/python3.6/site-packages/torch/testing/_internal/common_utils.py", line 888, in wrapper
Jan 14 05:43:21     method(*args, **kwargs)
Jan 14 05:43:21   File "test_jit_fuser_te.py", line 178, in test_zero_element_tensors_cuda
Jan 14 05:43:21     self._test_zero_element_tensors(device="cuda")
Jan 14 05:43:21   File "test_jit_fuser_te.py", line 174, in _test_zero_element_tensors

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.

@kurtamohler
Copy link
Copy Markdown
Collaborator Author

I suppose it's worth asking, should we also rename is_deterministic?

Also, this PR only changes the Python torch.set_deterministic function, it does not change the C++ interface. Should I change the C++ as well?

@kurtamohler kurtamohler force-pushed the use_deterministic_algorithms branch 5 times, most recently from 2ca48bc to 9762959 Compare December 29, 2020 03:11
@mruberry mruberry added module: determinism triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module labels Dec 29, 2020
Comment thread torch/__init__.py Outdated
Comment thread docs/source/torch.rst Outdated
Comment thread aten/src/ATen/Context.cpp Outdated
@kurtamohler
Copy link
Copy Markdown
Collaborator Author

Hey @mruberry, the name we chose, torch.is_deterministic_algorithms_enabled(), has been bothering me a little bit because it's not grammatically correct :P What do you think about torch.are_deterministic_algorithms_enabled()? I get that "is" is a much more common term for boolean state checking functions, but in this case, I think "are" feels more natural and could be easier for users to remember. What do you think?

@mruberry
Copy link
Copy Markdown
Collaborator

Hey @mruberry, the name we chose, torch.is_deterministic_algorithms_enabled(), has been bothering me a little bit because it's not grammatically correct :P What do you think about torch.are_deterministic_algorithms_enabled()? I get that "is" is a much more common term for boolean state checking functions, but in this case, I think "are" feels more natural and could be easier for users to remember. What do you think?

Sounds great.

@kurtamohler kurtamohler force-pushed the use_deterministic_algorithms branch from 45d2bf3 to 0fed3ed Compare January 14, 2021 00:30
@kurtamohler
Copy link
Copy Markdown
Collaborator Author

@mruberry , this PR is ready for another review. The ROCm failure doesn't seem to be related

@mruberry mruberry self-requested a review January 19, 2021 18:26
Copy link
Copy Markdown
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.

Awesome! Thanks @kurtamohler!

Copy link
Copy Markdown
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

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

@facebook-github-bot
Copy link
Copy Markdown
Contributor

@mruberry merged this pull request in 8ab1a14.

laurentdupin pushed a commit to laurentdupin/pytorch that referenced this pull request Apr 24, 2026
…#49904)

Summary:
Fixes pytorch#49100

Pull Request resolved: pytorch#49904

Reviewed By: ezyang, mrshenli

Differential Revision: D25956761

Pulled By: mruberry

fbshipit-source-id: 86a59289d50825a0ebbd7c358b483c8d8039ffa6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla signed Merged module: determinism open source triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Rename torch.set_deterministic for more clarity

4 participants