search

simonsobs / LAT_MFLike

Multifrequency Likelihood for SO Large Aperture Telescope
https://lat-mflike.readthedocs.io
Other
4 stars 12 forks source link

TE/ET fix #71

Closed beringueb closed 8 months ago

beringueb commented 8 months ago

Fixing the way cross-frequencies spectra are handled in MFLIKE. We noticed the T_nu1 x E_nu2 was stored in the same entry as T_nu2 x E_nu1 during theory building. While this is fine for CMB+FG, it isn't for calibration. Here is a quickfix, that explicitely store dls_dict[p, m["t2"], m["t1"]] (instead of dls_dict[p, m["t1"], m["t2"]]) in the case of 'ET' spectra.

We might want to do differently in the future but this should handle all the cases we can think of.

Also added some comments to document !

beringueb commented 8 months ago

Also not sure how to deal with versionining etc ... @xgarrido

xgarrido commented 8 months ago

For versionning, after the PR has been merged, we will create a new patch release v0.9.3 by changing this line https://github.com/simonsobs/LAT_MFLike/blob/master/setup.cfg#L2 and create a new release to push the version on pypi (this is something I'd like to change since versioneer is a bit deprecated or dead project and setuptools_scm better handles git version)

sgiardie commented 8 months ago

Hi Ben and all, I am trying to test a more general modification of mflike (+ syslibrary) where the polarization of the theory+FG spectra with systematics would be "TT/TE/ET/EE..." instead of "tt/te/ee...", which has been making the code a bit obscure and prone to mistakes. This requires a bit of work on both the mflike and syslibrary side, so if we want to merge this version very soon I can work on another branch.

beringueb commented 8 months ago

I would be keen on merging this now and explore other solutions in a separate branch ?

xgarrido commented 8 months ago

I don't really like having pending branches and PR that get outdated wrt the main branch. So, I suggest we merge this one (and I agree with @beringueb)