cosmodesi / desilike

BSD 3-Clause "New" or "Revised" License
20 stars 17 forks source link

consider removing copy of theories or making deepcopy? #10

Closed pmcdonal closed 1 year ago

pmcdonal commented 1 year ago

In this code (which is a simplified example to debug something that happened elsewhere), if I comment in the cosmoother lines (which, note, naively don't look like they should do anything), the final likelihood call dies horribly. If I use deepcopy it is fine. I understand copy vs. deepcopy in standard python, where sometimes there might be a reason for the distinction, but... does one ever need/want to make a non-deep copy here? (if yes, maybe the help could explain what it does...)(I admit, a lot of my problems probably stem from not being an instinctive python user)

from desilike import Fisher, setup_logging from desilike.likelihoods.cmb import BasePlanck2018GaussianLikelihood, TTHighlPlanck2018PlikLiteLikelihood, TTTEEEHighlPlanck2018PlikLiteLikelihood, TTLowlPlanck2018ClikLikelihood,\ EELowlPlanck2018ClikLikelihood, LensingPlanck2018ClikLikelihood from desilike.likelihoods import SumLikelihood from desilike.theories.primordial_cosmology import Cosmoprimo

setup_logging()

Planckavg={"h":0.6736,"omega_cdm":0.1200,"omega_b":0.02237,"logA":3.044,"n_s":0.9649, "tau_reio":0.0544} Planckbest={"h":0.6732,"omega_cdm":0.12011,"omega_b":0.022383,"logA":3.0448,"n_s":0.96605, "tau_reio":0.0543} cosmodefault = Cosmoprimo() cosmo=cosmodefault.copy()

cosmoother=cosmodefault.copy()

cosmo(**Planckbest)

cosmoother(**Planckavg)

likelihoods = [Likelihood(cosmo=cosmo) for Likelihood in [TTTEEEHighlPlanck2018PlikLiteLikelihood, TTLowlPlanck2018ClikLikelihood,\ EELowlPlanck2018ClikLikelihood, LensingPlanck2018ClikLikelihood]] likelihood_clik = SumLikelihood(likelihoods=likelihoods)

likelihood_clik()

adematti commented 1 year ago

Thanks for this actual bug report! it should be fixed now. Usually, indeed, you'll want to use deepcopy(). copy() is only useful in case one wants to duplicate a calculator, without copying its dependencies. If, e.g., we had a Theory giving the power spectrum at redshift z, given some cosmology cosmo:

theory1 = Theory(cosmo=cosmo, z=0.)

Then, if:

theory2 = theory.copy()
theory2.init.update(z=1.)

theory2 would use the same cosmo instance, but output the power spectrum at redshift z=1. If:

theory2 = theory.deepcopy()

then theory2 would use a new cosmo instance. I have updated the copy/deepcopy docstrings. Your feedback is really useful to improve things in this debugging phase, so please do not hesitate to ask if there is anything you'd like to do but does not seem straightforward.

pmcdonal commented 1 year ago

I didn't check the fix, but I guess this does make sense.