Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MFLike #183

Closed
cmbant opened this issue Jul 26, 2024 · 8 comments · Fixed by #186
Closed

MFLike #183

cmbant opened this issue Jul 26, 2024 · 8 comments · Fixed by #186

Comments

@cmbant
Copy link
Collaborator

cmbant commented Jul 26, 2024

Currently there are two different versions of MFLike (on separate git repos), SOLikeT and https://github.com/simonsobs/LAT_MFLike/, which is confusing and hard to maintain. As far as I can see, the other repo is more up to date, but the SOLikeT version has been converted into a set of Cobaya Theorys. However, not clear this gains anything in efficiency at the moment as there are no separate parameters for mflike (though in principle one could certainly split of e.g. calibration and foreground parameters). Can we just delete mflike from SOLikeT and pull from pypi? Or if we want the SOLikeT structure, merge refactor into LAT_MFLike?

(if they merge simonsobs/LAT_MFLike#80 it would obviously be good to have in SOLikeT for testing, both for dragging sampling and fast use with cosmopower)

Copy link
Collaborator

Hello Antony, yes, the idea behind the refactoring of MFLike into a set of Cobaya Theories was exactly to allow for possible independent parameters to be handled more efficiently, e.g., foregrounds and systematics (which might be shared by multiple likelihoods via fgspectra and syslib). Unfortunately, it has been always hard to stay up to date with the development of MFLike, which happens in the dedicated repo. My preference would be to keep the structure we have in SOLikeT (with improvements if needed), but I am open to re-discuss this if instead you/others think it is unnecessary

@cmbant
Copy link
Collaborator Author

cmbant commented Jul 29, 2024 via email

@cmbant
Copy link
Collaborator Author

cmbant commented Jul 31, 2024

I made a zeroth order attempt to merge the latest LAT_MFLike with the SOLikeT cobaya-theory structure (with ultimate objective of deleting from SOLikeT and importing). It didn't seem TheoryForge was really necessary once foregrounds were split off, so I merged it with mflike (they were mutually dependent anyway, and a lot of copy of attributes from one to the other, so don't think it was possible to use one without the other for systematics/calibrations?). For the moment I've simplified the number of theories so bandpasses are done as part of the BandpowerForeground class rather than separately. Calibration and nuisance parameters are now part of mflike, foreground and bandpass shifts are part of BandpowerForeground. BandpowerForeground seems to be all that is needed for pspipe to load the foreground model consistently as a separate class (without instantiating the likelihood).
Thoughts on structure welcome of this (very rough, untested) draft:

https://github.com/simonsobs/LAT_MFLike/tree/restructure

@cmbant
Copy link
Collaborator Author

cmbant commented Aug 1, 2024

Now PR simonsobs/LAT_MFLike#86. Unclear what exactly to do in SOLikeT - just delete mflike and corresponding tests, and assume it will be tested in the other repo? Or do we need to define an inherited class that multi-inherits from GaussianLikelihood?

@cmbant cmbant mentioned this issue Aug 2, 2024
@cmbant cmbant linked a pull request Aug 2, 2024 that will close this issue
@ggalloni
Copy link
Collaborator

ggalloni commented Aug 6, 2024

Hello @cmbant, I was thinking of possible alternative structures wrt to what you propose in simonsobs/LAT_MFLike#86.
How about something in between a TheoryForge and the new splitted-theories approach?

We could think of TheoryForge as an interface with a set of theories, which could be CAMB/CLASS plus Foreground (as they are now in your restructure branch) and something like "Systematics" containing for now all the operations on the output of the first two (calibration/rotation/templates). This means that each theory can have its parameters and TheoryForge will still provide a single set of spectra to MFLike as before so that the likelihood doesn't need to know about systematics and such. The interactions between these theories can be handled in TheoryForge of course.

This also implies that future development of systematics, or any of the other theories, can be done without touching the actual likelihood, which feels cleaner to me. Also, if someone for some reason wants to use a different version of some theory, that can be done easily through the interface.

Let me know what you think.

In the meantime, to test the idea and for fun, I tried to split also the systematics part as you did for foregrounds and it is kinda working (some devel is still needed).

@cmbant
Copy link
Collaborator Author

cmbant commented Aug 6, 2024

Thanks for looking at it. This sounds like the structure as it is now (pre-refactor) in SOLikeT? The way SoLikeT was written was quite logical and has some nice feature. But also had some slightly odd features (apart from being out of synch)

  • the calibration operation is trivial, but was made quite complicated
  • using the likelihoods gets cumbersome - have to define three theory classes and the likelihood
  • likewise but worse for instantiating separately as standalone components
  • the likelihood then depends on no parameters (maybe a feature not a bug...)
  • There is some overhead to splitting up into lots of components and passing things around, I haven't measured it, perhaps someone has - is it negligible?
  • More complicated to make e.g. TT, EE+TE versions (already not sure how to best do that... Likelihoods organization for different TT, TE, EE, TTTEEE modes  LAT_MFLike#3)

Another option would be to separate out systematics but leave calibration in likelihood (cf previous PRs).

@ggalloni
Copy link
Collaborator

ggalloni commented Aug 7, 2024

Yes indeed, I was sloppy, that is essentially what SOLikeT does, except for the systematics which are handled within TheoryForge. Probably I would separate those as you suggest in the end.
I agree with leaving calibration where it is.

For the single mode likelihoods, I opened a draft PR at simonsobs/LAT_MFLike#88. Locally tests are passing (pytest), I'll fix the ones failing there asap.
Let me know if there is something else to do about it and eventually we can include that into the restructure branch.

@cmbant
Copy link
Collaborator Author

cmbant commented Aug 7, 2024

Looking at the cobaya code, I don't think at the moment there's any way for a Theory component to dynamically provide a list of default sampled parameters based on what is sent to must_provide (at most, a class can dynamically construct the parameters based on its own input parameters via get_class_options). Not easy to change, as parameters are assigned based on inputs and class-level defaults, before classes are instantiated and dependencies and providers are determined. So probably to separate the different foreground parameters for TT, TE, TTTEEE etc would require making separate Foreground classes (in addition to separate mflike classes). The classes can of course inherit from each other/import yaml files with !default as for the Planck likelihoods to avoid duplication.

An alternative would be to specify input parameters to the Foreground theory, which would have to list which polarization components are needed, and get_class_options() could then respond accordingly.

(a default Foreground model could also be constructed as a "Help theory" by mflike, where each mflike variation made its own foreground theory variation helper class; this way users would not need to specify the foreground component manually (as for CAMB's helper theory class), but would make it less easy to change the foreground model independently).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants