CuArrays.jl
CuArrays.jl copied to clipboard
Add deterministic option
Some of the commonly used kernels (conv and maxpool) are not deterministic by default. This hurts reproducibility a lot - when fix random seed (e.g. by seed!(1)
) users cannot reproduce the results.
I added an option to provide default and check if a mode/algo is deterministic.
@maleadt Any comments on this?
This seems like a good idea, but I think we could do a slightly different setup:
- Have a global var for determinism, and provide getters and setters for this (like
isdeterministic
now) rather than requiringeval
- Document these and warn that some algo choices might be ignore if it is true
- If
isdeterministic
, force themode
to be a deterministic one regardless of what was passed as a kwarg argument, rather than throwing an error dependent onmode
.
How does that sound?
@MikeInnes I introduced the global variable for determinism and replaced assertions with warnings when not deterministic.
Looks fine to me, thanks. Would like a thumbs up from @maleadt.
I'm not against the feature, but I worry it will not be maintained and thus the determinism promise won't mean much. At the least needs some tests though, and it might be easier to test if the warning were just an error.
At the least needs some tests though, and it might be easier to test if the warning were just an error.
Sure I will add some tests.