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

Add spin from pulsar frequency to conversions #3432

Merged
merged 2 commits into from
Aug 26, 2020

Conversation

cdcapano
Copy link
Contributor

Adds a function to compute dimensionless spin given a pulsar's mass, radius, and rotation frequency. Just assumes the pulsar is a solid sphere when doing so. Example:

>>> from pycbc.conversions import spin_from_pulsar_freq
>>> spin_from_pulsar_freq(1.44, 11., 500.)
0.3975488979662235

Copy link
Contributor

@spxiwh spxiwh left a comment

Choose a reason for hiding this comment

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

This looks good, other than things codeclimate has already mentioned: Don't use "I" as a variable name. mt also doesn't pass PEP-8 conventions for variable names (at least 3 characters long and isn't really self-descriptive ... But we can find plenty of such examples within PyCBC code ).

Just need to make sure that any users are aware that this will just be an illustrative measure of the moment of inertia (because of the assumptions clearly stated in the help text). I can't think of any use case where that is a problem though!

@cdcapano cdcapano merged commit 1f3ae0a into gwastro:master Aug 26, 2020
@cdcapano cdcapano deleted the spin_from_freq branch August 26, 2020 14:45
lenona pushed a commit to lenona/pycbc that referenced this pull request Sep 14, 2020
* register the new function

* code climate
ArthurTolley pushed a commit to ArthurTolley/pycbc that referenced this pull request Nov 14, 2022
* register the new function

* code climate
OliverEdy pushed a commit to OliverEdy/pycbc that referenced this pull request Apr 3, 2023
* register the new function

* code climate
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