WIP Samplers should keep the same dtype as provided by theano.

[twiecki]

Related: #1246

[fhuszar]

Does this effect numerical precision when theano is not actually needed? Can we use model.dtypeinstead of theano.config.floatX? This way if the model does not actually need theano or to run on GPU, the sampling can still run in float64 even if theanorc specifies float32.

[twiecki]

How do you mean? Theano is always needed and I would expect that model.dtype must match theano.config.floatX.

[fhuszar]

Okay. Also, downcast only needs to happen at the very last step before the theano function is called. This is why I prefer the allow_input_downcast=True setting of theano.function as it only happens quietly when needed.

[twiecki]

I tried this over here #1265 but it doesn't seem to work.

[fhuszar]

hey, is a theano function involving logp recompiled every time a Metropolis-Hastings accept-reject happens? This is going to be painfully slow (although theano does cache some of the optimisations, you still only want to compile a function once).

[twiecki]

Actually this is just a python function (contrary to what I said earlier, I misremembered because we made delta_logp a theano function, and that function should only be compiled once).

[twiecki]

Probably can easily turn metrop_select into a function using ifelse. Then turn the proposal generation to theano random.

[twiecki]

Merged in #1265 in case it helps. It now seems this is the only way to enforce correct updating of shared variables.

[twiecki]

Can someone check if we can sample on the GPU now? @fhuszar

[cynddl]

It seems to work on my laptop (Python 3.5.1, Theano 0.8.2, PyMC3 from the branch use_theano_float_type). I tried the sample code from #1246 with both CPU and GPU. It returns 10k samples in 1.3s on CPU and 3.2s on GPU.

[twiecki]

@cynddl Awesome, thanks for checking!

[twiecki]

@cynddl A really good test would be https://github.com/twiecki/WhileMyMCMCGentlySamples/blob/master/content/downloads/notebooks/bayesian_neural_network_lasagne.ipynb (specifically the convolutional net at the bottom). Want to test with that?

[cynddl]

I've been trying with the branch use_theano_float_type for theano and the bleeding edge version for lasagne (I get a RuntimeError when defining models with the stable version). The sampling gets stuck at:

Iteration 0 [0%]: ELBO = -17251256.38

Same behavior on CPU or GPU. Maybe the problem is too large for my computer. Any idea?

[twiecki]

Yeah, possibly, it's definitely slow for such a big model. You could also try the smaller https://github.com/pymc-devs/pymc3/blob/master/docs/source/notebooks/bayesian_neural_network_advi.ipynb

[springcoil]

What's the status on this - are we working on a GPU - I need to get one for home so I can check this out. It looks some cool work - and it looks like a lot of the errors are fixed. I've not had time or the interest to look into this :)

[twiecki]

Yeah, I'm not quite sure what the best way to go about this is. If we enforce the same dtype everywhere model creation suffers. Maybe there is a way to only switch it on explicitly but that seems a bit cumbersome.

[Spaak]

@twiecki I'm somewhat confused. I think I'm running into this same error (TypeError: expected type_num 11 (NPY_FLOAT32) got 12) using either NUTS or Metropolis. When I look in the repository for metropolis.py, there is no code for dtype handling Yet PR #1253 is listed as merged into master, which should have added the relevant code.

Were these commits (inadvertently) reverted? Apologies if I'm missing something obvious.

(PS: I'm on CPU, not GPU, but with theano.config.floatX = 'float32'.)

[twiecki]

I did: 1c9adc6

I thought the PR fixed the problem even with trust_input. Apparently I was wrong. Can you try if removing that line fixes your problem?

[Spaak]

Like I wrote I think the commits in this PR (at least beb720e) have somehow disappeared from master. I'm not git wizard enough to try to determine how, but this seems to be the case. Because of that, here delta.dtype will be float64 independent of floatX setting, and thus q.dtype will be float64 as well.

Commenting the trust_input gives me, as expected, an error about Theano not wanting to downcast the input (upon calling self.delta_logp with q as input here). If I then add allow_input_downcast=True it all works of course, but of course that is suboptimal because we'd rather add in float32s in the first place.

[twiecki]

Hm, I reverted that merge because it caused some other unanticipated problems. Ideally we'd have dtypes consistent with floatX.

Just to make sure, you're saying that the model works with that commit in place?

[Spaak]

Yes, with that commit (or actually this one: a171012 because the other one is no longer reachable in the repository) in place the model works, even when 1c9adc6 is also in place.

[twiecki]

Interesting, I suppose we should revisit why other people where seeing problems with that in place.

[twiecki]

@Spaak Can you try this branch: #1338?

[Spaak]

Model works, both Metropolis and NUTS.

Edit: note, just for completeness, I'm running on CPU with floatX=float32, not GPU. Emphasizing this again because I saw an issue where CPU/GPU was relevant. Will get a GPU soon to test further.

Edit 2: that branch gives me massive dtype-related errors with ADVI, just sampling works well now.