Skip to content

Adding eccentric chirp mass in conversions.py - #5344

Merged
khunsang merged 8 commits into
gwastro:masterfrom
khunsang:master
Jun 18, 2026
Merged

Adding eccentric chirp mass in conversions.py#5344
khunsang merged 8 commits into
gwastro:masterfrom
khunsang:master

Conversation

@khunsang

@khunsang khunsang commented May 31, 2026

Copy link
Copy Markdown
Contributor

This PR adds functionality to compute the eccentricity-weighted chirp mass/eccentric chirp mass. The implementation introduces a new function in conversions.py. This papermeter is useful for estimating the noise-trigger rate of eccentric searches performed with TaylorF2Ecc. The parameter is derived analytically from the leading term of the TaylorF2Ecc phase expression in Eqs. 1.1, 3.3-3.6a of https://arxiv.org/abs/2108.05861. The eccentricity is defined as the time component of eccentricity ($e=e_t$) in quasi-Keplerian terms.

  • Limitations of the new parameter: Since the TaylorF2Ecc model is valid in a small eccentricity regime ($e \lesssim 0.2$), this new effective parameter is likewise expected to be valid only within the regime of validity of the TaylorF2Ecc model.

  • The author of this pull request confirms they will adhere to the code of conduct

@khunsang khunsang self-assigned this May 31, 2026
@ahnitz

ahnitz commented May 31, 2026

Copy link
Copy Markdown
Member

@khunsang Can you check if this is just a duplicate of #5276 or if there is functionality that should be merged together?

ahnitz
ahnitz previously requested changes May 31, 2026

@ahnitz ahnitz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Check on overlap with #5276

@khunsang

khunsang commented Jun 1, 2026

Copy link
Copy Markdown
Contributor Author

Hi @ahnitz, Thanks for having a look at the PR. I was not aware of #5276. Fortunately, I am not duplicating it. That PR is based on arXiv:2107.14736, where numerical fitting to time-frequency evolution was used to obtain the expressions. The expression I added is based on arXiv:2108.05861, which is analytically derived from the leading-order term of the TaylorF2Ecc phasing. This papermeter was particularly useful in computing noise trigger rate using the eccentric template bank in arxiv:2512.10803.

@ahnitz

ahnitz commented Jun 1, 2026

Copy link
Copy Markdown
Member

@khunsang Ok, great! It makes sense to have both. Do you have any opinion on how to keep the naming consistent, or should there be a flag or something to the same function to choose version? Thoughts?

Poke @rahuldhurkunde. Should we merge this one first and then update the other to somehow be consistent?

@ahnitz
ahnitz dismissed their stale review June 1, 2026 14:14

No overlap

@ahnitz ahnitz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@khunsang Beside how to maronize with #5276, can you update the documentation here in regards to the reference frequency for the eccentricity? There must be an implicit one in this definition, no?

@rahuldhurkunde

Copy link
Copy Markdown
Member

@ahnitz I'm fine with merging this first. However, as you said we need to converge on the naming conventions to include both the quantities.

Also, it would be helpful to compare the analytical and numerical expressions. I'll try to make some plots if no one else gets to it first.

@khunsang

khunsang commented Jun 2, 2026

Copy link
Copy Markdown
Contributor Author

Hi @rahuldhurkunde and @ahnitz, I don't think it's worthwhile to compare the two expressions. Bose and Pai's expressions reproduce Favata et al. in the low-eccentricity limit and at leading order ($e^2$) at a chirp mass of around 70 $M_\odot$. Bose and Pai's coefficients in their polynomial are mass dependent, whereas in Favata et al., the coefficient is constant. Both are derived from different perspectives; Fatava et al concern phase similarity (a quasi-circular SPA phase with chirp mass $\mathcal{M}{c,1}$ is similar to phase of an eccentric waveform with eccentric chirp mass $\mathcal{M}^{ecc}{c}(\mathcal{M}{c,2}, e_0) = \mathcal{M}{c,1}$, where $\mathcal{M}_{c,2}$ and $e_0$ are chirp mass and eccentricity of the second system). Whereas Bose and Pai are more about fitting the time-frequency evolution.

Regarding naming, could we use eccentric_chirp_mass_phase for Favata et al.'s expression and eccentric_chirp_mass_time_frequency for Bose and Pai expression ? I have also updated the documentation in the PR.

@ahnitz

ahnitz commented Jun 2, 2026

Copy link
Copy Markdown
Member

@khunsang @rahuldhurkunde If you can agree on names, that's fine with me. The other option is to modify Rahul's version to just be an option for this one in a second PR.

In either case, you'll want to rebase from master to make sure the tests run.

@khunsang

khunsang commented Jun 2, 2026

Copy link
Copy Markdown
Contributor Author

Hi @ahnitz @rahuldhurkunde , I agree in adding this function in Rahul's version as a special case.

@khunsang

khunsang commented Jun 3, 2026

Copy link
Copy Markdown
Contributor Author

Hi @ahnitz and @rahuldhurkunde, I have updated Rahul's version of the code, which can now compute Emchirp using both methods. I also renamed the function that I added to maintain consistency with the parameter naming convention proposed by Rahul.

@ahnitz

ahnitz commented Jun 3, 2026

Copy link
Copy Markdown
Member

@rahuldhurkunde Can you please do the review?

@khunsang @rahuldhurkunde One small thing, the name of the function contains a capital letter (and so do the options). Typically, the python convention is to use lower case in these scenarios.

@khunsang

khunsang commented Jun 3, 2026

Copy link
Copy Markdown
Contributor Author

Thanks, @ahnitz. Lower casing is done for methods; it would be for the functions once we converge on the parameter name. @rahuldhurkunde, your polynomials are now in Horner's form.

Comment thread pycbc/conversions.py Outdated
@rahuldhurkunde

Copy link
Copy Markdown
Member

Thanks @khunsang for incorporating the numerical fits into this PR. I'll say that changes from #5276 have been correctly pulled in. So I'm happy to sign-off on the fits method.

I've tried to compare the two methods and the results do not make sense. See the figure below:

Emchirp_scatter_parallel-imageonline co-merged

Can you please double check this?

Comment thread pycbc/conversions.py Outdated
Comment thread pycbc/conversions.py Outdated
@khunsang

khunsang commented Jun 5, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @rahuldhurkunde for the review and for comparing the two definitions. I updated the function names, changing the Emchirp to eccmchirp in all three functions.

@rahuldhurkunde

Copy link
Copy Markdown
Member

@khunsang Apologies I missed your comment: the two expressions are not worth comparing so I'm approving this now.

@rahuldhurkunde
rahuldhurkunde self-requested a review June 8, 2026 13:22
@khunsang
khunsang enabled auto-merge (squash) June 14, 2026 21:45
@khunsang
khunsang merged commit eb7ed1e into gwastro:master Jun 18, 2026
27 of 29 checks passed
SamInsley pushed a commit to SamInsley/pycbc that referenced this pull request Jul 21, 2026
* Added eccentric_mchirp

* added the reverse mchirp_from_ecc_mchirp_ecc

* bring back the cubic term, update the inverse function

* Adding eccentric chirp mass in conversions.py

* Adding SPA phase based eccentric chirp mass in Rahul's version

* Adding 'Emchirp_from_mass1_mass2_eccentricity' in '__all__'

* lower casing methods

* Renaming functions after Rahul's review

---------

Co-authored-by: rahuldhurkunde <rahuldhurkunde97@gmail.com>
Co-authored-by: Khun Sang Phukon <khun.phukon@ligo.org>
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.

3 participants