Skip to content

add deterministic flag + functionality #853

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

Conversation

alexander-soare
Copy link
Contributor

Follow up on discussion #845 (comment)

@rwightman
Copy link
Collaborator

@alexander-soare that still won't be deterministic no? torch.use_deterministic_algorithms(True) needs to be set as it has broader scope than cudnn flag. Also not sure if the benchmark mode needs to be disabled as well of uf use_deterministic_algorithms overrides that too...

@alexander-soare
Copy link
Contributor Author

alexander-soare commented Sep 7, 2021

@rwightman hmm yeah I must admit I'm not totally sure about this one. Empirically, it does what it's supposed to do. And using torch.use_deterministic_algorithms(True) raises issues with some algorithms not having determinism implemented (strange when considering my empirical success). I haven't dug too deeply into the docs to try to understand exactly what's going on. Same thing re. your benchmark comment - I tried without and it just works.

Happy to park this one until/if I get around to fully understanding what's going on.

@alexander-soare alexander-soare marked this pull request as draft September 8, 2021 07:14
@rwightman
Copy link
Collaborator

@alexander-soare deterministic is always such a bother :) so in your trials just the cudnn flag seemed to result in reproducible runs where as without it was different? It's definitely not a straightforward yay/nay... and yeah, the full flag it can break some models since not all ops have deterministic impl. I'll think about it, maybe make the deterministic setting a string/enum so one can do --deterministic cudnn, --deterministic full, etc

.. adding the flag and then not actually having it fully deterministic seems misleading, hah

@alexander-soare
Copy link
Contributor Author

@rwightman

so in your trials just the cudnn flag seemed to result in reproducible runs where as without it was different?

Correct.

And yeah makes sense to drop this for now. Besides, it's easy enough to diy for whoever wants it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants