Skip to content

Add cuDNN deterministic env variable (only for convolution) - #24747

Merged
tensorflow-copybara merged 2 commits into
tensorflow:masterfrom
duncanriach:cudnn_deterministic_v2
Jan 16, 2019
Merged

Add cuDNN deterministic env variable (only for convolution)#24747
tensorflow-copybara merged 2 commits into
tensorflow:masterfrom
duncanriach:cudnn_deterministic_v2

Conversation

@duncanriach

@duncanriach duncanriach commented Jan 7, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor

This change is a component of the recipe for making TensorFlow training reproducible on GPUs.

Setting the environment variable TF_CUDNN_DETERMINISTIC=1 (or true) will ensure that both forward and backwards convolution algorithms are both fixed and deterministic. It overrides autotune and selects deterministic back-prop algorithms.

This pull request has two previous abandoned versions:

  • pr/24301 was issued incorrectly based on r1.12.
  • pr/24355 was temporarily closed and could not be re-opened after a force-push to the branch.

This pull request is different from 24355 in the following ways:

  1. It caches the environment variable using a more favorable, pre-existing pattern.
  2. It uses tensor op math, if it's available.
  3. It factors the logic that decides if tensor op math is available into a separate inline function.
  4. If TF_CUDNN_DETERMINISTIC is set, the code that accumulates the list of algorithm options is skipped.

Attention @azaks2 @timshen91

@duncanriach duncanriach mentioned this pull request Jan 7, 2019
Closed
@Harshini-Gadige
Harshini-Gadige requested a review from jlebar January 7, 2019 22:00
@Harshini-Gadige Harshini-Gadige self-assigned this Jan 7, 2019
@Harshini-Gadige Harshini-Gadige added the awaiting review Pull request awaiting review label Jan 7, 2019
@jlebar
jlebar requested review from timshen91 and removed request for jlebar January 7, 2019 22:48
@jlebar

jlebar commented Jan 7, 2019

Copy link
Copy Markdown
Contributor

@timshen91 is a better reviewer for this, since he reviewed pr/24355.

timshen91
timshen91 reviewed Jan 7, 2019
View reviewed changes
Comment thread tensorflow/stream_executor/cuda/cuda_dnn.cc Outdated
Comment thread tensorflow/stream_executor/cuda/cuda_dnn.cc Outdated
@duncanriach

duncanriach commented Jan 8, 2019

Copy link
Copy Markdown
Contributor Author

Hi Tim (@timshen91), when I run clang-format on this file, I see formatting differences between the human and the program. Should all files be totally clean with clang-format? I think they're compliant with the coding guidelines, but the program made different choices. If so, should I issue another pull-request after this one to fix those formatting issues (so that other people don't have to pick through them)? What's the best practice on this?

@jlebar

jlebar commented Jan 8, 2019

Copy link
Copy Markdown
Contributor

You're seeing this because clang-format is not a stable format, and Google runs bleeding-edge clang, as compared to whatever version you happen to have installed on your system.

We've recently set things up so that when we import a PR, we run our bleeding-edge clang-format over the whole thing. So I believe @yifeif was planning to disable the clang-format check externally; it shouldn't be necessary anymore.

@jlebar

jlebar commented Jan 8, 2019

Copy link
Copy Markdown
Contributor

(So I'd say for this PR, don't worry about it.)

@tensorflowbutler tensorflowbutler removed the awaiting review Pull request awaiting review label Jan 8, 2019
@Harshini-Gadige Harshini-Gadige added awaiting review Pull request awaiting review size:S CL Change Size: Small labels Jan 8, 2019
@duncanriach
duncanriach force-pushed the cudnn_deterministic_v2 branch from db001c7 to 731ae19 Compare January 8, 2019 22:17
@duncanriach

duncanriach commented Jan 9, 2019

Copy link
Copy Markdown
Contributor Author

I've made some changes. This is ready for review again.

@Harshini-Gadige Harshini-Gadige added kokoro:force-run Tests on submitted change awaiting testing (then merge) and removed awaiting review Pull request awaiting review labels Jan 9, 2019
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Jan 9, 2019
@duncanriach

duncanriach commented Jan 10, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

I'm assuming these four build failures are unrelated to my pull request. Please let me know if I caused this and/or if there is anything I need to to to run those checks again.

@Harshini-Gadige Harshini-Gadige added ready to pull PR ready for merge process and removed awaiting testing (then merge) labels Jan 10, 2019
@Harshini-Gadige

Harshini-Gadige commented Jan 10, 2019

Copy link
Copy Markdown

I'm assuming these four build failures are unrelated to my pull request. Please let me know if I caused this and/or if there is anything I need to to to run those checks again.

I'm helping to get this PR merged. I'll keep you posted if anything is required from your end.

@tensorflow-copybara
tensorflow-copybara merged commit 731ae19 into tensorflow:master Jan 16, 2019
tensorflow-copybara pushed a commit that referenced this pull request Jan 16, 2019
@tensorflower-gardener
Merge pull request #24747 from duncanriach-nvidia:cudnn_deterministic_v2
c6f456c 
PiperOrigin-RevId: 229457378
@duncanriach duncanriach mentioned this pull request Jan 29, 2019
Merged
@duncanriach

duncanriach commented Jan 29, 2019

Copy link
Copy Markdown
Contributor Author

See follow-on pull request 25269 that addresses non-determinism in max pooling.

@duncanriach duncanriach mentioned this pull request Feb 16, 2019
Merged
@j-towns j-towns mentioned this pull request Apr 3, 2019
Closed
@duncanriach duncanriach mentioned this pull request Jun 11, 2019
Merged
@mrgloom

mrgloom commented Jun 30, 2019

Copy link
Copy Markdown

How it's related to TF_CUDNN_USE_AUTOTUNE ?
From here
TF_CUDNN_DETERMINISTIC used to disable auto-tuning and select deterministic cuDNN convolution algorithms.

@duncanriach

duncanriach commented Jul 1, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@mrgloom, TF_CUDNN_USE_AUTOTUNE=false disables TF cuDNN auto-tuning, which is enabled by default. TF cuDNN auto-tuning tries different forward and backward algorithms for each layer to find the highest-performing one.

TF_CUDNN_DETERMINISTIC=true selects deterministic algorithms, where they are available, and ensures that the same algorithms are always used, even when there are only deterministic options available. This is what is meant by disabling auto-tuning: there is no automatic algorithm selection.

When TF_CUDNN_DETERMINISTIC=true is used TF_CUDNN_USE_AUTOTUNE=false should not be used. The latter is not only unnecessary but will actually thwart the effect of the former.

@duncanriach duncanriach mentioned this pull request Aug 9, 2019
Merged
@duncanriach duncanriach mentioned this pull request Oct 8, 2019
Closed
@duncanriach
duncanriach deleted the cudnn_deterministic_v2 branch October 17, 2019 20:25
@duncanriach duncanriach mentioned this pull request Dec 9, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@timshen91 timshen91 timshen91 approved these changes

Assignees

@Harshini-Gadige Harshini-Gadige

Labels

cla: yes ready to pull PR ready for merge process size:S CL Change Size: Small

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@duncanriach @jlebar @Harshini-Gadige @mrgloom @timshen91 @googlebot @kokoro-team @tensorflowbutler @tensorflow-copybara