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

ENH: gravitational waves artale rates #474

Open
wants to merge 23 commits into
base: module/gravitational_waves
Choose a base branch
from

Conversation

itrharrison
Copy link
Contributor

@itrharrison itrharrison commented Jul 19, 2021

Description

Adds a new model for compact binary merger rates to the gravitational wave(s) module, from Artale et al (2020).

There are three models, depending cumulatively in each of stellar mass, star formation rate and metallicity. Each model is a simple logarithmic model with coefficients for the log of each independent variable and a constant.

The values of the parameters are fit to objects in the EAGLE simulation suite, with independent fits on simulation output at four redshifts: 0.1, 1, 2, 6. At the moment a simple linear interpolation between these points is performed for the coefficients as a function of redshift.

ToDo

  • Finish implementing and documenting SFR and metallicity-dependent models
  • Expand tests
  • Decide on consistent treatment of units
  • Improve treatment of redshift dependence (function decorators?)

Checklist

  • Follow the Contributor Guidelines
  • Write unit tests
  • Write documentation strings
  • Assign someone from your working team to review this pull request
  • Assign someone from the infrastructure team to review this pull request

@itrharrison itrharrison self-assigned this Jul 19, 2021
@ntessore ntessore changed the base branch from main to module/gravitational_waves July 19, 2021 12:46
@ntessore ntessore force-pushed the module/gravitational_waves-artale-rates branch from 09cbf83 to a57d7b1 Compare July 19, 2021 12:54
@itrharrison itrharrison force-pushed the module/gravitational_waves-artale-rates branch from a57d7b1 to c5e1007 Compare July 19, 2021 12:58
@itrharrison itrharrison marked this pull request as ready for review July 21, 2021 11:03
@itrharrison itrharrison requested a review from a team as a code owner July 21, 2021 11:03
@Lucia-Fonseca
Copy link
Member

@itrharrison could you please add ENH:(enhancement) at the beginning of this PR's title? Thank you.

@itrharrison itrharrison changed the title Module/gravitational waves artale rates ENH: gravitational waves artale rates Jul 22, 2021
@itrharrison itrharrison added the enhancement Improvement of existing feature label Jul 22, 2021
skypy/gravitational_waves/merger_rate.py Outdated Show resolved Hide resolved
skypy/gravitational_waves/merger_rate.py Show resolved Hide resolved
skypy/gravitational_waves/merger_rate.py Outdated Show resolved Hide resolved
skypy/gravitational_waves/merger_rate.py Show resolved Hide resolved
skypy/gravitational_waves/merger_rate.py Show resolved Hide resolved
skypy/gravitational_waves/merger_rate.py Outdated Show resolved Hide resolved
skypy/gravitational_waves/tests/test_merger_rate.py Outdated Show resolved Hide resolved
skypy/gravitational_waves/tests/test_merger_rate.py Outdated Show resolved Hide resolved
skypy/gravitational_waves/tests/test_merger_rate.py Outdated Show resolved Hide resolved
Copy link
Member

@Lucia-Fonseca Lucia-Fonseca left a comment

Choose a reason for hiding this comment

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

A few minor details, erase examples and question on private functions.

skypy/gravitational_waves/merger_rate.py Outdated Show resolved Hide resolved
skypy/gravitational_waves/merger_rate.py Outdated Show resolved Hide resolved
skypy/gravitational_waves/merger_rate.py Outdated Show resolved Hide resolved
skypy/gravitational_waves/merger_rate.py Outdated Show resolved Hide resolved
skypy/gravitational_waves/merger_rate.py Outdated Show resolved Hide resolved
skypy/gravitational_waves/merger_rate.py Outdated Show resolved Hide resolved
skypy/gravitational_waves/merger_rate.py Outdated Show resolved Hide resolved
skypy/gravitational_waves/merger_rate.py Outdated Show resolved Hide resolved
skypy/gravitational_waves/merger_rate.py Outdated Show resolved Hide resolved
skypy/gravitational_waves/merger_rate.py Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvement of existing feature module: gravitational wave
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants