CobayaSampler / cobaya

Code for Bayesian Analysis
http://cobaya.readthedocs.io/en/latest/
Other
126 stars 125 forks source link

Allow sigma8 as input for CAMB #301

Closed tilmantroester closed 1 year ago

tilmantroester commented 1 year ago

This PR allows specifying sigma8 as the normalisation of the power spectrum when using the CAMB theory code. The use case is large-scale structure analyses that sample in sigma8 instead of As. An example is the KiDS-1000 analysis, which samples in S8 to avoid implicit priors on that parameter.

For now this only works with power law initial power spectra, where As can be rescaled and power spectra recomputed after the transfer functions have been computed.

The current implementation seems to do what it's supposed to but I'm not very familiar with cobaya, so if there's a better approach to achieve this, I'd be happy to adapt the implementation.

cmbant commented 1 year ago

Thanks, interesting, I'll have to think about it. (It's obviously rather inefficient to calculate everything from time transfer functions twice.)

cmbant commented 1 year ago

I can't think of a better way of doing it, and looks fine to me and useful for checking (though I would not encourage use given extra numerical cost and arbitrariness of sigma8!). Does it raise an error if someone provides both As and sigma8 as input?

You might be able to do the initial power law check somewhere in initialization instead of once running (initialize_with_params?).

codecov-commenter commented 1 year ago

Codecov Report

Merging #301 (1a33173) into master (d98444d) will increase coverage by 0.02%. The diff coverage is 100.00%.

:exclamation: Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@            Coverage Diff             @@
##           master     #301      +/-   ##
==========================================
+ Coverage   84.81%   84.84%   +0.02%     
==========================================
  Files         122      122              
  Lines        8917     8934      +17     
==========================================
+ Hits         7563     7580      +17     
  Misses       1354     1354              
Impacted Files Coverage Δ
cobaya/theories/camb/camb.py 90.79% <100.00%> (+0.34%) :arrow_up:
tilmantroester commented 1 year ago

I've added a check that raises an error if both As and sigma8 are inputs. I also moved what was in set to initialise_with_params.

I agree that there are good reasons for not using sigma8 as a parametrisation of the power spectrum amplitude but for better or worse, that's what we're stuck with for low-redshift stuff...

cmbant commented 1 year ago

Thanks for the PR

cmbant commented 1 year ago

I made a few tweaks, e.g. moving power law check out of run loop and checking also for As. please check https://github.com/CobayaSampler/cobaya/commit/4a52113b1a3084f8f35a3945931df1fd2a221a5f

tilmantroester commented 1 year ago

Looks reasonable to me. Is the reason for using initialize_with_provider that needs_perts might get changed after initialize_with_params? Thanks for reminding me that the := exists, btw!

cmbant commented 1 year ago

Thanks. Yes, it only knows what it's been asked to calculate after all dependencies resolved, which is after initialize_with_params; initialize_with_provider is the last step before actually running a calculation