search

brinckmann / montepython_public

Public repository for the Monte Python Code
MIT License
93 stars 79 forks source link

Bug in need_cosmo_arguments() affecting BK14 and BK15 #275

Closed ThomasTram closed 1 year ago

ThomasTram commented 2 years ago

Hi

Since v3.5, the BK14 and BK15 likelihoods (and perhaps others) no longer work. This is due to a bug introduced in commit d00543211545c2c08b13baa2716778c75de5d8f1 that modified the need_cosmo_arguments() function:

https://github.com/brinckmann/montepython_public/blob/4f010ad7ede7306ada811de80cad8369dbd6338f/montepython/likelihood_class.py#L248

The problem is that the logic inside the function was changed such that it relies on num_flag being initialised to True at the beginning of each iteration, not just at the beginning of the function. Thus, if a non-numeric value precedes a numeric value, the code will execute data.cosmo_arguments[key] = 0 but not the next data.cosmo_arguments[key] = value statement, so for BK14, we end up setting l_max_tensors to zero!

It would be great if someone started implementing a set of tests inside a GitHub action such that new PRs are not breaking previous functionality...

Cheers, Thomas

dchooper commented 1 year ago

I have implemented this bugfix, it will be included in MP v3.6.0 (coming soon!). Thanks for pointing it out!

Having tests would definitely be a great addition, if someone has time and wants to start that and do a PR, that would be great!