simonsobs / LAT_MFLike

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

Issue with TE and ET cross-frequencies spectra #70

Closed beringueb closed 8 months ago

beringueb commented 8 months ago

@xgarrido , @sgiardie and I found that the way mflike deals with TE and ET for cross frequency spectra is not clear and possibly not correct at the moment.

Possible fixes include:

HTJense commented 8 months ago

Would a potential solution be to just be more consistent in swapping the keys in the dls_dict when distinguishing between TE/ET? We could change this check to

if m["hasYX_xsp"]:  # not symmetrizing
    dls_dict[p, m["t2"], m["t1"]] = cmbfg_dict[p, m["t2"], m["t1"]]
else:
    dls_dict[p, m["t1"], m["t2"]] = cmbfg_dict[p, m["t1"], m["t2"]]

(note the swapped indices on the left-hand side of the first case assignment). In other words, store E_1 x T_2 as T_2 x E_1 (which should be equivalent) in the dls_dict (which should, in my opinion, be simpler to read as well than adding an extra key in the dls_dict and ignoring half the possible entries).

The only other change needed then would be to add a check in the mflike._get_power_spectra function, to check that

for m in self.spec_meta:
    p = m["pol"]
    i = m["ids"]
    w = m["bpw"].weight.T
    dls_obs = DlsObs[p, m["t1"], m["t2"]]
    if m["hasYX_xsp"]: dls_obs = DlsObs[p, m["t2"], m["t1"]]
    cls = w @ dls_obs
    ps_vec[i] = clt

To make sure we also access the same entry.

beringueb commented 8 months ago

Yes, I think this would work too and might be simpler to implement and understand. I've done it in the TE_ET_quickfix branch.

@sgiardie can you look at the TE_ET_quickfix branch and see if it works ?

sgiardie commented 8 months ago

Hi Ben, if you use Hidde's solution then the extra flag m["hasYX_xsp"] should be removed from dls_dict[p, m["t1"], m["t2"], m["hasYX_xsp"]] in theoryforge

sgiardie commented 8 months ago

and also clt = w @ DlsObs[p, m["t1"], m["t2"]] --> clt = w @ dls_obs here

beringueb commented 8 months ago

Yes, sorry went a bit too fast here. Fixed now

sgiardie commented 8 months ago

The code works and now it makes sense how it handles the cross frequency spectra in the case without symmetrization. I have checked it makes sense also in the case with symmetrization. I added some comments in the code to make it a bit more transparent. We should decide if we want to stick with this version or make the code even clearer by setting the polarizations to TT/TE/ET/EE etc (instead of the camb ones tt/te/ee...) and eliminating the need of the flag "hasXY_xsp".

xgarrido commented 8 months ago

Fix https://github.com/simonsobs/LAT_MFLike/pull/71