search

nespinoza / juliet

A versatile modelling tool for transiting and non-transiting (single and multiple) exoplanetary systems
MIT License
53 stars 31 forks source link

Bug fix while using different params for different instruments for `fp`, `p` and `p1` #112

Closed Jayshil closed 7 months ago

Jayshil commented 7 months ago

Hi @nespinoza,

The current version of juliet (master and dev branches, which I believe is the latest stable version) has a bug. While fitting for different parameters for fp, p and p1 for different instruments, the sampling will happen only for the last instruments in the list. For example, while fitting for fp_p1_instrument1 and fp_p1_instrument2, the sampling will happen only for the later parameter and the prior would return as posterior for the former.

This is because of a missing if statement while creating self.fp_iname, self.p_iname and self.p1_iname dictionaries in __init__ function of the model class (see, changes below to see what I mean). Because of this missing if statement, those self.fp_iname[p1][ins] etc. dictionaries were kept updating for each ins in the loop and the final value of self.fp_iname[p1][ins] would always correspond to the last instrument in the loop. This PR fixes this issue by adding an if statement. Let me know if you have any questions on this.

Cheers, Jayshil

nespinoza commented 7 months ago

Thanks for this catch! Merging...

nespinoza commented 7 months ago

Hey @Jayshil,

I took a deeper look after merging in dev on this, and I remembered what happened here. The problem with your fix is that this fix makes juliet incompatible with previous versions, in which you could just define a prior on p_p1 for instance to fit a wavelength-independant planet-to-star radius ratio.

Just a heads-up that I'm working on this so the final juliet code might look a bit different on this part than your PR.

N.

Jayshil commented 7 months ago

Hi @nespinoza,

Thanks for informing me! I did not realise that the extra else statement there was for juliet back-compatibility. The changes you made now are a more clear way of doing this. Thanks for doing this!

Cheers, Jayshil