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

Euclid requisites -- CAMB & CLASS #222

Merged
merged 52 commits into from
Feb 22, 2022
Merged

Conversation

JesusTorrado
Copy link
Contributor

@JesusTorrado JesusTorrado commented Dec 14, 2021

  • Omega_b|cdm|nu_massive(z)
    • CAMB
    • CLASS
    • add test
  • extrap_kmin for P(k,z) interpolator (common for CLASS and CAMB)
  • angular_diameter_distance_2
    • pooling and requesting values
    • CAMB
    • CLASS
    • add test
  • CLASS: sigma8(z) -- restore test
  • CLASS: fsigma8(z) -- restore test
  • try to incorporate add sigma_R to classy (WIP) #95
  • CLASS: Pk_Weyl -- test
  • CLASS: Remove Pk_max 1.5 factor
  • CLASS: last neutrino case fails in GUI
  • CLASS: update to 3.1.X (devel for now)
  • Update CHANGELOG

Other:

  • added --minimize flag to cobaya-run

@JesusTorrado JesusTorrado marked this pull request as draft December 14, 2021 21:31
@codecov-commenter
Copy link

codecov-commenter commented Dec 16, 2021

Codecov Report

Merging #222 (7da498b) into master (36a9acc) will increase coverage by 0.31%.
The diff coverage is 91.34%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #222      +/-   ##
==========================================
+ Coverage   87.54%   87.85%   +0.31%     
==========================================
  Files          92       92              
  Lines        8058     8333     +275     
==========================================
+ Hits         7054     7321     +267     
- Misses       1004     1012       +8     
Impacted Files Coverage Δ
cobaya/input.py 86.03% <ø> (ø)
cobaya/likelihoods/base_classes/des.py 83.87% <ø> (ø)
cobaya/likelihoods/base_classes/sn.py 94.55% <ø> (ø)
cobaya/log.py 89.41% <ø> (ø)
cobaya/model.py 94.63% <ø> (+0.89%) ⬆️
cobaya/mpi.py 91.56% <ø> (ø)
cobaya/output.py 87.77% <ø> (ø)
cobaya/parameterization.py 91.13% <ø> (ø)
cobaya/prior.py 96.92% <ø> (+0.76%) ⬆️
cobaya/samplers/mcmc/proposal.py 97.19% <ø> (ø)
... and 14 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 36a9acc...7da498b. Read the comment docs.

@JesusTorrado JesusTorrado changed the title Euclid requisites -- CAMB Euclid requisites -- CAMB & CLASS Feb 1, 2022
cobaya/tools.py Outdated Show resolved Hide resolved
cobaya/tools.py Outdated Show resolved Hide resolved
cobaya/tools.py Show resolved Hide resolved
cobaya/theories/camb/camb.py Outdated Show resolved Hide resolved
cobaya/theories/classy/classy.py Show resolved Hide resolved
cobaya/theories/classy/classy.py Outdated Show resolved Hide resolved
cobaya/tools.py Show resolved Hide resolved
cobaya/theories/camb/camb.py Outdated Show resolved Hide resolved
args = args[:i_z] + [z_pool.values] + args[i_z:]
else:
raise LoggedError(
self.logger,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

self.log

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, didn't notice that one, even though the linter complains. At some point, I'd like to change all the log attributes to logger, which is more precise. And maybe the raise LoggedError to a method?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A method would be nice, but I think complicates the stack dump unless you hack it somehow

@cmbant
Copy link
Collaborator

cmbant commented Feb 4, 2022

Could do with some more tests of new code?

@CobayaSampler CobayaSampler deleted a comment from cmbant Feb 4, 2022
@JesusTorrado
Copy link
Contributor Author

Almost done here (just a segfault in CLASS needing fixing).

  • We are not using this global k_max anywhere at the moment, right? Could I remove it?

    - ``k_max=[...]``: Fixes the maximum comoving wavenumber considered.

  • When adding sigma_R tests, I've noticed that we return x, y, A[y,x], which is a bit weird. Do you think sigma_R is used little enough that we can still change that? (I mean, transpose A)

@cmbant
Copy link
Collaborator

cmbant commented Feb 15, 2022

Yes, looks like we are not using k_max. CAMB has "kmax" which can be set to force higher kmax. Sigma_R order doesn't bother me, but could make it y, x, A(y,x) I suppose; not sure if anyone uses it.

@JesusTorrado
Copy link
Contributor Author

OK, I'll change it. Any preference for either z, R, sigma(z,R) or R, z, sigma(R,z)? I guess the second.

Feel free to review if/whenever you want. As soon as the new CLASS release (3.1.2) is out, I could merge.

@cmbant
Copy link
Collaborator

cmbant commented Feb 15, 2022

I think sigma(z,R) is the "natural" order from camb. I think I has a quick look at most things last review, anything specific new I should look at?

@cmbant
Copy link
Collaborator

cmbant commented Feb 15, 2022

Do a check for unused imports; minimize argument for run not in docstring.

@cmbant
Copy link
Collaborator

cmbant commented Feb 17, 2022

_get_z_dependent has different arguments in camb and base, should probbaly be consistent.

@JesusTorrado
Copy link
Contributor Author

CLASS has been updated, so this can be merged. Some notes:

  • _get_z_dependent indeed has different arguments. CAMB has 2 different pools of z values, and the way the code is written right now, CAMB's _get_z_dependent is in practice a wrapper over the general Boltzmann one that passes the right pool. I could give CAMB's re-implemented method a dummy pool argument that is never used if that's making your linter squeak (but it's a private method anyway).
  • get_sigma_R now returns z, R, sigma(z,R).
  • Unused imports removed (I think)
  • minimize added to run docstring

I don't think there is anything left to check.

@JesusTorrado JesusTorrado marked this pull request as ready for review February 22, 2022 22:10
@JesusTorrado JesusTorrado merged commit d26e833 into master Feb 22, 2022
@JesusTorrado JesusTorrado deleted the euclid_requisites_camb branch March 29, 2023 18:41
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 this pull request may close these issues.

3 participants