bayesiains / nflows

Normalizing flows in PyTorch
MIT License
845 stars 118 forks source link

Added option for inverse autoregressive flows & corresponding unit test #16

Closed rbvh closed 4 years ago

rbvh commented 4 years ago

Hello,

I made some simple modifications to the autoregressive transform such that it also allows one to use the architecture set out in 1606.04934. That is, the original implementation is fast in the inference direction but slow in the sampling direction, but now you can choose which direction is fast.

I actually started implementing this because I originally incorrectly assumed the prior implementation was slow in the inference direction, but now that I wrote it I figured it might as well go in.

Cheers, Rob

arturbekasov commented 4 years ago

Hi Rob,

Thanks for taking the time to submit the PR -- very much appreciated.

A quick question before I dive deeper into the code: when we wanted a fast sampling / slow inference AR layer before (e.g. to use as an approximate posterior in a VAE), we'd do the following:

fast_inference_ar = AutoregressiveTransform(...)
fast_sampling_ar = InverseTransform(fast_inference_ar)

My hunch is that the new code might be equivalent to this. What are your thoughts?

Thanks,

Artur

rbvh commented 4 years ago

Hi Artur,

You're right, that seems to accomplish exactly the same thing without the extra code. Sorry! I'll close the pull request

arturbekasov commented 4 years ago

No worries at all!

In fact, this showcases that the solution is useful and yet poorly-documented. A PR with updates to documentation (probably the docstring of AutoregressiveTransform in particular) would be very welcome. :-)

Artur

On 26 Aug 2020, at 14:49, rbvh notifications@github.com wrote:

 Hi Artur,

You're right, that seems to accomplish exactly the same thing without the extra code. Sorry! I'll close the pull request

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.