Skip to content

add rational fits#223

Merged
vijayvarma392 merged 51 commits into
mainfrom
rational_fit
Oct 10, 2025
Merged

add rational fits#223
vijayvarma392 merged 51 commits into
mainfrom
rational_fit

Conversation

@md-arif-shaikh
Copy link
Copy Markdown
Collaborator

@md-arif-shaikh md-arif-shaikh commented Sep 25, 2024

Add rational_fit as an alternative method to interpolate the omega at the extrema.
This PR introduces a new option in extra_kwargs for omega_gw_extrema_interpolation_method which defaults to the spline-based interpolation.

omega_gw_extrema_interpolation_method can be set to either of the following values:

  • spline: Uses InterpolatedUnivariateSpline with k=3 (as default).
  • rational_fit: Uses rational approximation as implemented in polyrat.StabilizedSKRationalApproximation

rational_fit improves the fit to the omega_gw at pericenter and apocenters, particularly near the merger where spline struggles to remain monotonic.

@md-arif-shaikh md-arif-shaikh marked this pull request as draft September 25, 2024 09:38
@md-arif-shaikh
Copy link
Copy Markdown
Collaborator Author

This PR is quite long, so I want to highlight the main changes

  • utils.py: general rational fit functions added here
  • eccDefinition.py: rational fit functions for fit to the omega_gw_extrema added. This is the most important function to be reviewed.

Other changes including the tests are straightforward. I Had to change a few things to make the tests pass because we have changed the default to rational_fit. More changes are to be made to the tests but that can be done later in a separate PR.

@md-arif-shaikh
Copy link
Copy Markdown
Collaborator Author

Hi @vijayvarma392, this PR is now ready to be reviewed. I have summarized the main changes here. I think at this point, I would like to have your comments for making further changes.

Comment thread gw_eccentricity/eccDefinition.py
Comment thread gw_eccentricity/eccDefinition.py Outdated
Comment thread gw_eccentricity/eccDefinition.py Outdated
Comment thread gw_eccentricity/eccDefinition.py Outdated
Comment thread gw_eccentricity/eccDefinition.py Outdated
Comment thread gw_eccentricity/eccDefinition.py Outdated
Comment thread gw_eccentricity/eccDefinition.py Outdated
Comment thread gw_eccentricity/eccDefinition.py Outdated
Comment thread gw_eccentricity/eccDefinition.py Outdated
Comment thread gw_eccentricity/eccDefinition.py Outdated
Comment thread gw_eccentricity/eccDefinition.py
Comment thread gw_eccentricity/eccDefinition.py Outdated
Comment thread gw_eccentricity/eccDefinition.py Outdated
Comment thread gw_eccentricity/eccDefinition.py Outdated
Comment thread gw_eccentricity/eccDefinition.py Outdated
Comment thread gw_eccentricity/eccDefinition.py Outdated
Comment thread gw_eccentricity/eccDefinition.py Outdated
Comment thread gw_eccentricity/eccDefinition.py
Comment thread gw_eccentricity/eccDefinition.py Outdated
Comment thread gw_eccentricity/eccDefinition.py Outdated
Comment thread gw_eccentricity/eccDefinition.py Outdated
Comment thread gw_eccentricity/eccDefinition.py Outdated
Comment thread gw_eccentricity/eccDefinition.py Outdated
Comment thread gw_eccentricity/eccDefinition.py Outdated
Comment thread gw_eccentricity/eccDefinition.py
Comment thread gw_eccentricity/eccDefinition.py Outdated
Comment thread gw_eccentricity/eccDefinition.py Outdated
A dictionary of kwargs for `omega_gw_extrema_interpolation_method`.
Currently, the available `omega_gw_extrema_interpolation_method` and
the corresponding defaults for `special_interp_kwargs_for_omega_gw_extrema`
are:
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Maybe this can be clearer:
"Currently, the available omega_gw_extrema_interpolation_method and the corresponding defaults for special_interp_kwargs_for_omega_gw_extrema are" -->

The defaults are set according to the value of `omega_gw_extrema_interpolation_method`:"

 - if omega_gw_extrema_interpolation_method is "spline", default kwargs are set using `utils.get_default_spline_kwargs`
 - if omega_gw_extrema_interpolation_method is "rational_fit", ...

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Done

f"is {kwargs['extrap_order']}."}
if any([kwargs["include_zero_ecc"], kwargs["include_params_dict"],
not kwargs["keep_memory"]]):
not kwargs.get("keep_memory", True)]):
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Hmm, are we keeping memory by default? Maybe better to do the opposite?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

No, the default value is False. So by default not kwargs.get("keep_memory", True)]) will be True and therefore one requires to give path to metadata. The default value (the secoond arg True) to kwargs.get() is to ensure that if someone is loading old format sxs waveform where "keep_memory" is not a kwarg, not kwargs.get() will evaluate to False, meaning no requirement to provide metadata file.

Md Arif Shaikh added 2 commits October 7, 2025 08:52
@vijayvarma392 vijayvarma392 merged commit 72e9b12 into main Oct 10, 2025
3 checks passed
@vijayvarma392 vijayvarma392 deleted the rational_fit branch October 10, 2025 13:25
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