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

split flrw protocol into pure and standard #18

Merged
merged 1 commit into from
Jan 20, 2023
Merged

split flrw protocol into pure and standard #18

merged 1 commit into from
Jan 20, 2023

Conversation

nstarman
Copy link
Contributor

Signed-off-by: nstarman [email protected]

Description

This pull request is to address ...

Fixes #

PR Checklist

  • Give a detailed description of the PR above.
  • Document changes in the CHANGES.rst file. See existing changelog for examples.
  • Add tests, if applicable, to ensure code coverage never decreases.
  • Make sure the docs are up to date, if applicable, particularly the docstrings and RST files in docs folder.
  • Ensure linear history by rebasing, when requested by the maintainer.

@nstarman nstarman added this to the v0.1 milestone Jan 20, 2023
@ntessore
Copy link
Contributor

Thinking more about the naming, and what we really want to achieve with this API.

I like the "standard" name, we could go even further and call it "StandardCosmology" without qualifier. I think that fits the spirit of the interface, which doesn't follow a strict model delineation like LCDM, wCDM, w0waCDM, etc. but instead simply has all the "standard" components.

Along the same lines, the idea behind "FLRWCosmology" is also not so much about strict adherence to the FLRW model, but rather to have a smaller interface that deals purely with the general-relativistic background equations. So one could and say that the FLRW interface could be called "BackgroundCosmology".

Trying to apply that taxonomy to the codes on my desk, it would work quite well. For example, I can immediately see three use cases:

  1. A function that does pure FLRW background level stuff, e.g. computing quantities from distance functions etc. Such a function might call isinstance(cosmo, BackgroundCosmologyAPI), which I think shows the requirement on the interface quite clearly.

  2. A function that does "standard model of cosmology" computations and needs quantities such as e.g. the baryon fraction. Such a function might call isinstance(cosmo, StandardCosmologyAPI), which I think makes it clear that we have a more specific requirement there.

  3. A function doing e.g. weak gravitational lensing, which depends on distance functions, but also $\Omega_m \delta_m$. This is clearly beyond pure background, because we have $\Omega_m$, so we would say we need StandardCosmologyAPI rather than BackgroundCosmologyAPI. But there are also non-standard cosmological models with a modified lensing sector, in which it is not simply $\Omega_m \delta_m$ doing the lensing. In such a function we might therefore have a conditional such as

    if isinstance(cosmo, StandardCosmologyAPI):
        factor = cosmo.Om0
    elif isinstance(cosmo, ModifiedGravityCosmologyAPI):  # just an example
        factor = ...

    which once again shows the requirements on the interface very clearly in the code.

I am not saying we should adopt these names necessarily, but there seems to be a clear semantic difference between the kinds of interfaces we want to provide, and I think we should make this distinction immediately obvious to users.

@nstarman
Copy link
Contributor Author

Merging because this is blocking some other PRs. We can change the exact names later.

@nstarman nstarman merged commit 27573c5 into main Jan 20, 2023
@nstarman nstarman deleted the split-flrw branch January 20, 2023 23:54
@nstarman
Copy link
Contributor Author

My GH keeps on not updating every when I refresh. I missed this comment.

@ntessore
Copy link
Contributor

No problem, I think we sent our respective comments at the exact same second. Mine is more of a meta-comment than a comment on this PR specifically.

@nstarman
Copy link
Contributor Author

nstarman commented Jan 20, 2023

Followup PR: I like the names Background and Standard. I'll move Ok and H0 from the former to the latter, leaving only Otot

@nstarman
Copy link
Contributor Author

This way we can stick intermediate classes between Background and Standard, e.g. with inhomogeneity, where a global curvature parameter is less meaningful.

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.

2 participants