simonsobs / LAT_MFLike

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

Draft single-mode MFLikes #88

Closed ggalloni closed 2 months ago

ggalloni commented 3 months ago

Draft PR for single-mode MFLikes.

This is essentially doing what @xgarrido proposed in #3. Is there something else to do for this?

cmbant commented 3 months ago

I think it depends how much we want to tidy up the parameter dependence. I think this sets all the same parameters for all the components? (so e.g. the EE likelihood still depends on the fixed-default T calibration param; but more importantly, also probably by default sampling a_kSZ that it is actually unused)

cmbant commented 3 months ago

Btw you can inherit parameters/yaml, so e.g. do not need to duplicate most of the yaml content - can have once in base class and then just define "defaults: polarizations: " etc as needed in derived classes (either in yaml or as inherited class attributes).

Can also use imports in yamls, e.g. see the NPIPE likelihoods in cobaya (params: !defaults ...)

codecov-commenter commented 3 months ago

Codecov Report

Attention: Patch coverage is 96.47059% with 3 lines in your changes missing coverage. Please review.

Project coverage is 85.33%. Comparing base (3e669e7) to head (3c58c01).

Files Patch % Lines
mflike/mflike.py 91.66% 2 Missing :warning:
mflike/foreground.py 98.27% 1 Missing :warning:
Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/simonsobs/LAT_MFLike/pull/88/graphs/tree.svg?width=650&height=150&src=pr&token=qrrVcbNCs5&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=simonsobs)](https://app.codecov.io/gh/simonsobs/LAT_MFLike/pull/88?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=simonsobs) ```diff @@ Coverage Diff @@ ## restructure #88 +/- ## =============================================== + Coverage 83.93% 85.33% +1.40% =============================================== Files 3 3 Lines 417 457 +40 =============================================== + Hits 350 390 +40 Misses 67 67 ``` | [Files](https://app.codecov.io/gh/simonsobs/LAT_MFLike/pull/88?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=simonsobs) | Coverage Δ | | |---|---|---| | [mflike/\_\_init\_\_.py](https://app.codecov.io/gh/simonsobs/LAT_MFLike/pull/88?src=pr&el=tree&filepath=mflike%2F__init__.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=simonsobs#diff-bWZsaWtlL19faW5pdF9fLnB5) | `71.42% <100.00%> (ø)` | | | [mflike/foreground.py](https://app.codecov.io/gh/simonsobs/LAT_MFLike/pull/88?src=pr&el=tree&filepath=mflike%2Fforeground.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=simonsobs#diff-bWZsaWtlL2ZvcmVncm91bmQucHk=) | `89.41% <98.27%> (+1.21%)` | :arrow_up: | | [mflike/mflike.py](https://app.codecov.io/gh/simonsobs/LAT_MFLike/pull/88?src=pr&el=tree&filepath=mflike%2Fmflike.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=simonsobs#diff-bWZsaWtlL21mbGlrZS5weQ==) | `82.75% <91.66%> (+1.23%)` | :arrow_up: |
ggalloni commented 3 months ago

I fixed the calibration part, so, for instance, EE doesn't need calT to be there, not even fixed. I did this after your suggestion just by modifying the yamls. However, doing the same for the foreground params is far more complex.

The issue is the initialization step, where Foreground doesn't know anything about MFLike, or, at least, I could not make them communicate on that step (after that it can be done). I guess this is what you meant in #3 by "dynamically vary the nuisance parameters", am I right?

The only thing that worked is the workaround I'm pushing after cc27f0f. Essentially, Foreground and MFLike have all the fg parameters, but each mode-specific likelihood has a supported_params attribute. Then, I force the unsupported params to be fixed to some default value (I am using the ones from the test, but ideally these would be the best-fits of some run I guess).

I know it is not much different than before, but at least this forcefully avoids unsupported params being sampled...

Do you know a way to make components communicate at the initialization step somehow? Or maybe re-trigger some part of the initialization part after that (assignment of parameters, adding a drop tag to the unsupported ones, something like that)?

cmbant commented 3 months ago

See comment here https://github.com/simonsobs/SOLikeT/issues/183

cmbant commented 2 months ago

Extending this, my attempt to make params match dynamically: https://github.com/simonsobs/LAT_MFLike/pull/89. It does require specifying "requested_cls" to the foreground model consistently, but should at least raise an error if it is inconsistent. Or can use TTForeground etc. classes.

ggalloni commented 2 months ago

I merged #89 into this PR since I don't have write permissions on the other side. Then, I added TT+EE, TT+TE, and TE+EE keeping requested_cls on the yamls.

cmbant commented 2 months ago

Looks good to me!