search

jluastro / PopSyCLE

Population Synthesis for Compact-object Lensing Events
https://popsycle.readthedocs.io/en/latest/
16 stars 3 forks source link

Birth kicks #10

Closed samrose30 closed 4 years ago

samrose30 commented 4 years ago

Hello everyone! I think the birth_kicks branch should be ready to merge back into master. This branch changes the arguments of perform_pop_syn from specifying a single kick speed for the NSs and BHs to specifying the mean of a maxwellian distribution from which kick speeds are drawn from randomly for each NS or BH formed. The default value for NSs is 400 km/s which comes from Hobbs et al 2005 'A statistical study of 233 pulsar proper motions'. The default value for BHs does not have a reference but is set at 50 km/s.

MichaelMedford commented 4 years ago

I took care of resolving the conflicts with master to have the correct variables for NS and BH mean kick speed arguments passed to functions. This branch looks to me to be ready to be merged into master.

MichaelMedford commented 4 years ago

Yup, it looks like in attempting to fix the conflicts I messed up the function arguments. Sorry about that! I'll fix that now.

MichaelMedford commented 4 years ago

I have now fixed the merge issues, and fixed a couple of typos in the docstring.

MichaelMedford commented 4 years ago

I think the comment here is not quite clear. Don't we mean that if you set this parameter, you force the seed to the user specified value? The default should be set to 42.

This should no longer be an issue, given the other discussion about setting random seeds.

MichaelMedford commented 4 years ago

@jluastro @caseylam @samrose30 All issues have been addressed. I'm thinking that one of the approvers (Jessica or Casey) should have to be the one to complete the merge pull request, so as to make sure that the changes are sufficient.