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.

Sign up for GitHub

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

Jump to bottom

ENH: gravitational waves artale rates #474

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

ENH: gravitational waves artale rates #474

wants to merge 23 commits into from

Conversation

itrharrison
Copy link
Contributor

@itrharrison itrharrison commented Jul 19, 2021 •
edited

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
@itrharrison itrharrison added this to In progress in Gravitational Waves via automation 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
philipp128
philipp128 requested changes Jul 22, 2021
View reviewed changes
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
skypy/gravitational_waves/tests/test_merger_rate.py Show resolved Hide resolved
Gravitational Waves automation moved this from In progress to Review in progress Jul 22, 2021
Lucia-Fonseca
Lucia-Fonseca reviewed Jul 22, 2021
View reviewed changes
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
itrharrison and others added 6 commits July 22, 2021 15:26
@itrharrison @Lucia-Fonseca
Update skypy/gravitational_waves/merger_rate.py
5cfcf4e 
Co-authored-by: Lucia F. de la Bella <55983939+Lucia-Fonseca@users.noreply.github.com>
@itrharrison @Lucia-Fonseca
Update skypy/gravitational_waves/merger_rate.py
de63b1c 
Co-authored-by: Lucia F. de la Bella <55983939+Lucia-Fonseca@users.noreply.github.com>
@itrharrison @philipp128
Update skypy/gravitational_waves/tests/test_merger_rate.py
94d16d3 
Co-authored-by: philipp128 <48715661+philipp128@users.noreply.github.com>
@itrharrison @Lucia-Fonseca
Update skypy/gravitational_waves/merger_rate.py
31e69f8 
Co-authored-by: Lucia F. de la Bella <55983939+Lucia-Fonseca@users.noreply.github.com>
@itrharrison @Lucia-Fonseca
Update skypy/gravitational_waves/merger_rate.py
e8afd1d 
Co-authored-by: Lucia F. de la Bella <55983939+Lucia-Fonseca@users.noreply.github.com>
@itrharrison @Lucia-Fonseca
Update skypy/gravitational_waves/merger_rate.py
c713d81 
Co-authored-by: Lucia F. de la Bella <55983939+Lucia-Fonseca@users.noreply.github.com>
itrharrison and others added 8 commits July 22, 2021 15:27
@itrharrison @Lucia-Fonseca
Update skypy/gravitational_waves/merger_rate.py
bfac3de 
Co-authored-by: Lucia F. de la Bella <55983939+Lucia-Fonseca@users.noreply.github.com>
@itrharrison @Lucia-Fonseca
Update skypy/gravitational_waves/merger_rate.py
2464202 
Co-authored-by: Lucia F. de la Bella <55983939+Lucia-Fonseca@users.noreply.github.com>
@itrharrison @Lucia-Fonseca
Update skypy/gravitational_waves/merger_rate.py
97cab80 
Co-authored-by: Lucia F. de la Bella <55983939+Lucia-Fonseca@users.noreply.github.com>
@itrharrison @Lucia-Fonseca
Update skypy/gravitational_waves/merger_rate.py
c322ffa 
Co-authored-by: Lucia F. de la Bella <55983939+Lucia-Fonseca@users.noreply.github.com>
@itrharrison @Lucia-Fonseca
Update skypy/gravitational_waves/merger_rate.py
21b2f19 
Co-authored-by: Lucia F. de la Bella <55983939+Lucia-Fonseca@users.noreply.github.com>
@itrharrison
removed private functions, doc examples
15f951d
@itrharrison
couple fo small typos
b7e15f9
@itrharrison
changed docs to specify astropy.Quantity
cd58d76
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Lucia-Fonseca Lucia-Fonseca Lucia-Fonseca left review comments

@ntessore ntessore Awaiting requested review from ntessore

@laurawolz laurawolz Awaiting requested review from laurawolz

@philipp128 philipp128 Awaiting requested review from philipp128

Requested changes must be addressed to merge this pull request.

Assignees

@itrharrison itrharrison

Labels
enhancement Improvement of existing feature module: gravitational wave
Projects
Gravitational Waves
  
Review in progress
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@itrharrison @Lucia-Fonseca @philipp128