cosmodesi / desilike

BSD 3-Clause "New" or "Revised" License
21 stars 16 forks source link

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

Open pmcdonal opened 1 year ago

pmcdonal commented 1 year ago

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)

adematti commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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.