Skip to content

New effective eccentric mchirp parameter#5276

Open
rahuldhurkunde wants to merge 4 commits intogwastro:masterfrom
rahuldhurkunde:Eccmchirp
Open

New effective eccentric mchirp parameter#5276
rahuldhurkunde wants to merge 4 commits intogwastro:masterfrom
rahuldhurkunde:Eccmchirp

Conversation

@rahuldhurkunde
Copy link
Copy Markdown
Member

Added a new eccentric mchirp parameter as defined in arXiv:2107.14736.

Standard information about the request

This is a: new feature

This change affects: the offline search.

This change changes:result presentation / plotting

Motivation

A new effective mchirp-e parameter might be better for eccentric searches, i.e. may lead to better bank placement.

Contents

Added a new method in conversions.py

Testing performed

Reproduced the Figure 3. in the paper.

ecc_mchirp_scatter_parallel

Additional notes

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

@rahuldhurkunde rahuldhurkunde self-assigned this Feb 3, 2026
Comment thread pycbc/conversions.py Outdated
return mass2_from_mtotal_eta(mtotal, eta)


def ecc_mchirp_from_mchirp_ecc(mchirp, ecc):
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.

@rahuldhurkunde Nice. I might suggest two things.

(1) (minor) is there a better name than 'ecc mchirp'?
(2) Is it possible to write an inverse function? e.g. give back the mchirp from the 'ecc mchirp' + ecc.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@ahnitz The inverse function requires a root solver. So to avoid adding a scipy dependency, I had to include a new bisection root solver function: solve_root.

Simple testing seems to be working:

>>> conversions.ecc_mchirp_from_mchirp_ecc(20.0, 0.2)
20.668174895333745
>>> conversions._mchirp_from_ecc_mchirp_ecc(20.668174895333745, 0.2)
19.999999648528163

Sorry I can't think of any better name for the new parameter. Happy to change if you have any suggestions.

Copy link
Copy Markdown
Member Author

@rahuldhurkunde rahuldhurkunde Feb 10, 2026

Choose a reason for hiding this comment

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

@ahnitz I've dropped the e^6 term, simplified the inversion to a quadratic one and renamed the new parameter to Emchirp.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I've bring back the cubic term, and performed some quick testing on 50000 points. In short, both the functions work well and this looks ready to merge from my end.

--- Testing Conversions for 50000 points ---
Using ranges from plot_ecc_mchirp.py:
  ecc: [0.0, 0.6]
  ecc_mchirp: [5.0, 80.0]

Timing (total for 50000 points):
  mchirp_from_ecc_mchirp_ecc (Inverse): 0.003358 s
  ecc_mchirp_from_mchirp_ecc (Forward): 0.002018 s

Validity Check (Round-trip):
  Number of NaNs in recovered mchirp: 0
  Max absolute difference in mchirp: 2.13e-14
  Mean absolute difference in mchirp: 6.26e-16

  RESULT: SUCCESS - The inverse function is valid in this range.

@yi-fan-wang
Copy link
Copy Markdown
Member

@rahuldhurkunde I have a PR #5328, where I try to enable fitting the noise trigger rate with respect to eccentricity. I think it'd be good to think about adding eccentricity mchirp as well

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