-
Notifications
You must be signed in to change notification settings - Fork 68
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
Container for MG params #1119
Container for MG params #1119
Conversation
Pull Request Test Coverage Report for Build 5743350713
💛 - Coveralls |
@tilmantroester , I'm not sure it's worth trying to make this a CCLNamedClass. This is more appropriate for things like halo model ingredients, baryonic effects, or emulators, where there are a gazillion parametrisations and you want to sift through them quickly. |
Fair enough. Changing this wouldn't affect the API so this could be made a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @tilmantroester . Looks good to me, just a couple of minor things before merging.
Are there opinions on the spelling of "parametrisation"? Do we use AE spelling and use z instead of s? |
I think AE for the most part (although I find myself being very inconsistent) |
Switched to "parametrization" now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
This PR introduces a
ModifiedGravity
interface to theCosmology
class which encapsulates MG parametrisations (just mu-Sigma for now). Under the hood (i.e. C-layer) nothing has changed.The motivation is to reduce clutter in the
Cosmology
interface and allow different MG parametrisations in the future without breaking the API.Instead of
the new API is now
This doesn't use
CCLNamedClass
at the moment due to a conflict withdataclass
(see #1118).