brinckmann / montepython_public

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

v3.5 bug in need_cosmo_arguments #299

Closed EBHolm closed 1 year ago

EBHolm commented 1 year ago

The function need_cosmo_arguments of the standard likelihood class in likelihood_class.py was changed in version 3.5. This introduced a bug, where if an inheriting likelihood calls need_cosmo_arguments with a dictionary that has entries of both numerical and non-numerical type, the numerical values may not be set.

The variable num_flag is initialized on line 248 in likelihood_class.py before the loop over dictionary entries. If the first dictionary entry is non-numerical, num_flag is set to False within the first iteration of the loop. However, even if the second entry is numerical, num_flag is still False from the last iteration, and the numerical value will not be read.

The fix is straightforward: Simply move the initialization of num_flag inside the loop over dictionary entries.

I believe this bug was introduced in the commit 4f010ad7ede7306ada811de80cad8369dbd6338f which modified the structure of need_cosmo_arguments. It occurs only when using likelihoods that call need_cosmo_arguments with a dictionary that has numerical values appearing after a non-numerical value, such as the PyBird likelihoods, which call it with the input {'output': 'mPk', 'z_max_pk': self.z, 'P_k_max_h/Mpc': 1.}.

brinckmann commented 1 year ago

Thank you Emil, for the bug report and the proposed fix and pull request!

Best, Thejs

brinckmann commented 1 year ago

Note to self: this looks related to issue #276 and associated pull request. To everyone else affected, sorry for not getting this incorporated yet!

EBHolm commented 1 year ago

Hi Thejs,

Thank you for your response! This is exactly identical to issue #276, yes! Sorry, I had not seen that before making this issue.

Best, Emil