nanograv / QuickCW

Fast continuous wave (CW) analysis for PTA data
https://quickcw.readthedocs.io
MIT License
7 stars 5 forks source link

MCMC proposal outside prior raises `FloatingPointError` and kills sampler #12

Open astrolamb opened 4 months ago

astrolamb commented 4 months ago

On line 6 of runQuickMCMC.py, there's a line that changes the default behaviour of numpy when 0.0 is input into np.log. The default behaviour is a warning. Line 6 changes the bevaiour to raising a FloatingPointError. This causes an error when an MCMC iteration proposes a value outside of a prior. Instead of assigning a logpdf of -np.inf, as expected of an MCMC sampler, it just kills the sampler.

https://github.com/nanograv/QuickCW/blob/d7e12cbf929506b300b58414a23f27829c3644cc/runQuickMCMC.py#L6

I see that there are similar lines of code throughout QuickCW that have been commented out. I assume this line should've also have been commented out?

I raise this because it was a source of confusion for a student who thought it was actually an enterprise problem initially.

bencebecsy commented 1 month ago

Thanks for raising this. The thinking behind np.seterr(all='raise') was that we'd rather have the sampler fail than something that might be an actual problem go unnoticed as a warning.

Now this is clearly not the case for a proposal outside the prior, but I actually think that should not happen, because we reflect back all parameters to be in the prior range before getting prior and likelihood with functions in the QuickCorrectionUtils.py file. But maybe that does not work in some corner cases.

@astrolamb , do you happen to know what parameter was outside the prior range in this case?

astrolamb commented 1 month ago

@bencebecsy the issue happened when @levischult tried to run some QuickCW analyses in April. I've tried using their old scripts but I can't recreate the bug, but those scripts/data could've been fixed somehow between then and now. This might need some further testing