Support deterministic upsample trilinear backward #154239
Conversation
isuruf
requested review from
albanD,
jbschlosser and
mikaylagawarecki
as code owners
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/154239
Note: Links to docs will display an error until the docs builds have been completed. ✅ You can merge normally! (1 Unrelated Failure)As of commit 7da28c1 with merge base 53057fc ( UNSTABLE - The following job is marked as unstable, possibly due to flakiness on trunk:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
torch/nn/functional.py
Outdated
| # are_deterministic_algorithms_enabled. | ||
| if not torch.jit.is_scripting(): | ||
| if torch.are_deterministic_algorithms_enabled() and ( | ||
| input.is_cuda or input.is_xpu |
There was a problem hiding this comment.
is not input.is_cpu what we want ?
torch/nn/functional.py
Outdated
| # Two levels are necessary to prevent TorchScript from touching | ||
| # are_deterministic_algorithms_enabled. | ||
| if not torch.jit.is_scripting(): | ||
| if torch.are_deterministic_algorithms_enabled() and ( |
There was a problem hiding this comment.
Not that it super matters, but the determinstic_algorithms call is slower than the not input.is_cpu. should we switch them ?
import timeit; import torch as t; x = t.tensor([1]); print(f"are_deterministic_algorithms_enabled: {timeit.timeit(lambda: t.are_deterministic_algorithms_enabled(), number=1000000):.6f}s vs is_cpu: {timeit.timeit(lambda: not x.is_cpu, number=1000000):.6f}s")
are_deterministic_algorithms_enabled: 0.108508s vs is_cpu: 0.062671s
I guess for cpu, it's better for the deterministic_algorithms_enabled call to go first, and for cuda the other way.. it's fine as is just thinking aloud.
| ): | ||
| # Use slow decomp whose backward will be in terms of index_put | ||
| # importlib is required because the import cannot be top level | ||
| # (cycle) and cannot be nested (TS doesn't support) |
|
@pytorchbot merge |
Merge startedYour change will be merged once all checks pass (ETA 0-4 Hours). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
Merge failedReason: 1 jobs have failed, first few of them are: trunk / win-vs2022-cpu-py3 / test (default, 3, 3, lf.windows.4xlarge.nonephemeral) Details for Dev Infra teamRaised by workflow job |
| ) | ||
| if input.dim() == 5 and mode == "trilinear": | ||
| assert align_corners is not None | ||
| # Two levels are necessary to prevent TorchScript from touching |
There was a problem hiding this comment.
I guess this will also still fail with TS? (which is fair, just want to confirm that the fallback below will fail somehow gracefully :D
There was a problem hiding this comment.
I'm not sure. I kept the same structure as the upsample bilinear code path.
|
@pytorchbot rebase -b viable/strict |
|
@pytorchbot started a rebase job onto refs/remotes/origin/viable/strict. Check the current status here |
|
Successfully rebased |
|
@pytorchbot merge |
Merge startedYour change will be merged once all checks pass (ETA 0-4 Hours). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
Stack from ghstack (oldest at bottom):
Fixes #154183