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

clarify how likelihood gets it's initial parameter values from theory #11

Open
pmcdonal opened this issue Feb 1, 2023 · 4 comments
Open
Labels
documentation Improvements or additions to documentation

Comments

@pmcdonal
Copy link
Member

pmcdonal commented Feb 1, 2023

E.g.,
Planckbest={"h":0.6732,"omega_cdm":0.12011,"omega_b":0.022383,"logA":3.0448,"n_s":0.96605,
"tau_reio":0.0543}
cosmobest=Cosmoprimo(**Planckbest)
for key,val in Planckbest.items(): cosmobest.varied_params[key].update(value=val)

Without the last line, when I plug this cosmobest into likelihood it spits back likelihood value corresponding to Cosmoprimo(), when I would have expected it to pick up Planckbest... which it does if I include the last line.

As often happens, I realize at this point I could just not worry about this and do likelihood(**Planckbest) to initialize... but I've written this and the point seems still valid...

All of this would be unnecessary if there was a simple function "lnL_Planck()" which returned Planck likelihood function (in DESI default combination) with no need to think about cosmo. Yes, if you want to combine with other things you need to input (or pull out) cosmo, but why not keep simple things simple (i.e., not deal with it if not necessary for fisher_planck)

@pmcdonal pmcdonal added the documentation Improvements or additions to documentation label Feb 1, 2023
@adematti
Copy link
Member

adematti commented Feb 1, 2023

The behavior you describe is expected: calling any calculator with **params does not change parameter's default values.

For you last remark, you can instantiate any Planck likelihood without specifying 'cosmo', e.g.:

likelihood = TTTEEEHighlPlanck2018PlikLiteLikelihood()
likelihood(logA=3.)

This is generically the case for any calculator, there are (almost?) always relevant default options; e.g.

theory = LPTVelocileptorsTracerPowerSpectrumMultipoles()
theory(logA=3.)

returns ells = (0, 2, 4) multipoles.
I guess you meant, it'd be good to have this for the sum of Planck (log-)likelihoods? In this case, you can write a simple function that returns this:

def PlanckFullLikelihood():
    cosmo = Cosmoprimo(fiducial='DESI')
    likelihoods = [Likelihood(cosmo=cosmo) for Likelihood in [TTTEEEHighlPlanck2018PlikLiteLikelihood, TTLowlPlanck2018ClikLikelihood, EELowlPlanck2018ClikLikelihood, LensingPlanck2018ClikLikelihood]]
    return SumLikelihood(likelihoods=likelihoods)

@pmcdonal
Copy link
Member Author

pmcdonal commented Feb 1, 2023

Yes, I thought after writing that I should have checked if just leaving off cosmo option worked. This is all great, but I would just say that fisher_planck.ipynb could be a lot clearer if you just wrote this combined function and called it at the top instead of introducing the theory that plays no real role in this notebook. I know, you probably have the reasonable thought that this doubles as a tutorial on assembling likelihoods, but... it confused me. (I often have the problem that, you "expose" so many apparent options that I have trouble figuring out what is the important thing/thing I want (where I am probably still somewhat befuddled by the fact that in python you can do "dir(object)" and start trying to call/use anything you see there that looks potentially relevant... I guess I should rely more heavily on documentation)

But about the original point: I don't know why you wouldn't want the likelihood to inherit the current settings of parameters of a theory (as its own current settings (these "current settings" of parameters of objects that are really functions are always a little... conceptually weird)), but in any case it would be good if help said how it works...

@adematti
Copy link
Member

adematti commented Feb 1, 2023

on the last point: likelihood does inherit cosmo settings (as you saw, doing cosmobest.all_params[key].update(value=val) propagates to likelihood). But desilike doesn't consider the parameters where you just called e.g. cosmo as settings... just "current value of parameters."

@pmcdonal
Copy link
Member Author

pmcdonal commented Feb 1, 2023

I don't understand this last sentence, but maybe it isn't important as I was putting too much weight on tinkering with theory before setting up likelihood.

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

No branches or pull requests

2 participants