search

gwnrtools / nr-catalog-tools

A unified interface to various catalogs of Numerical Relativity simulations of compact binary mergers.
https://github.com/gwnrtools/nr-catalog-tools
GNU General Public License v3.0
1 stars 5 forks source link

Bugfix generate polarizations #26

Closed vaishakp closed 1 year ago

vaishakp commented 1 year ago

This commit adds a feature and fixes some bugs in the computation of the waveform polarizations.

Feature add:

  1. Inherit WaveformModes.filepath and WaveformModes.sim_metadata from catalog for SXS.
  2. Presently, MAYA data does not provide the NR t_ref but instead Omega. Added method to compute this from Omega and the l2m2 orbital phasing.

Changes:

  1. Move the computation of reference orbital phase from lvc to waveforms.

Bug fix:

  1. fixed variable mismatch in get_td_waveform method.
prayush commented 1 year ago

@vaishakp - this needs a pull and rebase to resolve conflicts

vaishakp commented 1 year ago

@vaishakp - this needs a pull and rebase to resolve conflicts

On pull and rebase, the pull request is not getting updated. This is probably due to the fact that my fork now contains commits from another repo (streamline imports). Should I create another PR, or just resolve them directly here without a pull and rebase?

vaishakp commented 1 year ago

@vaishakp - this needs a pull and rebase to resolve conflicts

On pull and rebase, the pull request is not getting updated. This is probably due to the fact that my fork now contains commits from another repo (streamline imports). Should I create another PR, or just resolve them directly here without a pull and rebase?

It turns out the reason why this PR was not updating was that github was down yesterday. This can be now be merged.

prayush commented 1 year ago

@vaishakp the oldest commit of this PR 9755781639417e6de3380697be865a5eedb897dc - has already been merged into master. Why is it showing up here?

vaishakp commented 1 year ago

@vaishakp the oldest commit of this PR 9755781639417e6de3380697be865a5eedb897dc - has already been merged into master. Why is it showing up here?

I think this is the default behaviour of github when squash + merge is selected. When rebasing from the common ancestor commit, github does not update its PR wrt the changes in the target branch. I don't know about the implications.

Should we try not squashing the commits?

nvm, I will reopen this PR in #31 .