tensorflow / probability

Probabilistic reasoning and statistical analysis in TensorFlow
https://www.tensorflow.org/probability/
Apache License 2.0
4.27k stars 1.1k forks source link

Bug: parallel_iterations inside sample_halton_sequence can be None #87

Closed jonas-eschle closed 6 years ago

jonas-eschle commented 6 years ago

Software (tf & tfp): newest, compared with github.

In sample_halton_sequence(), a seed can be specified. Default is None, will be passed on as None and won't be altered (also not by util.gen_new_seed, this returns None if the argument was already None). On line 291 (sample_halton_sequence.py), tf.map_fn is called with the argument

parallel_iterations=1 if seed[0] is not None else seed[0]

So if the seed is None, then parallel_iterations will be None. And that's what it is. This then fails, as tf.map_fn calls while_loop, which requires parallel_iterations to be a positive integer (and not None).

Is the quoted line above really the intention? Should None be the number of parallel_iterations? (If not None -> 1 makes somewhat sense (reproducibility?), but if None, then it should not be None but something different I guess)

I am not familiar enough with the parallel_iterations handling, otherwise I'd be happy to make a PR and fix it.

saxena-ashish-g commented 6 years ago

The intent here (I think) was to let the parallel iterations be whatever the default is for the map_fn op when the seed has not been specified (and hence reproducibility is not an issue) but to fix it to 1 if the seed has been set. I will fix this shortly.

jvdillon commented 6 years ago

I believe this issue is fixed. Please reopen if Im mistaken.