pyro icon indicating copy to clipboard operation
pyro copied to clipboard

Fix #3255 (draft)

Open gui11aume opened this issue 2 years ago • 3 comments

gui11aume avatar Aug 30 '23 22:08 gui11aume

The purpose of this pull request is to harmonize the behavior of masking between the different ways of estimating the gradient of the ELBO (most notably when has_r_sample is False). See the discussion on issue #3255.

I added some tests inspired from those that were already present. Just ignoring the fact that some sites of the model are not present in the guide gave the correct results right away. The main difficulties were:

  1. Having missing sites in the guide triggered a warning that made the tests fail.
  2. The memory blew up with 50,000 samples in one particular case (going above 256 GB).

I had to disable user warnings for test_mask_gradient and break down sampling in 10 series of 5000 each.

The changes to pyro itself are otherwise as discreet as possible.

gui11aume avatar Aug 30 '23 23:08 gui11aume

@gui11aume can you please fix lint issues from make lint output and push changes? You can first run make format to automatically fix formatting and then manually fix any leftover linting issues.

ordabayevy avatar Oct 04 '23 03:10 ordabayevy

Hi @ordabayevy! Thanks for taking the time to help me with this. There were issues in the file tests/infer/test_gradient.py (mostly in the code for the new tests) and running make format seemed to fix them all. Let me know if some issues remain.

gui11aume avatar Oct 04 '23 22:10 gui11aume