Skip to content
Snippets Groups Projects

Simplify spin transformation for NR waveforms

Closed Michael Boyle requested to merge michael.boyle/lalsuite:simplify-spin-rotation into master

This commit eliminates the temporary "helper" timeseries and a corresponding loop over times by noting that each step in that loop is equivalent to multiplying the quantity (hplus - I * hcross) by exp(-2*I*alpha). This can be seen either by explicitly checking the math in the current code or just from the nature of spin-weighted functions. Specifically, this chunk of code is essentially setting

(hplus - I * hcross) = (hplus_corr - I * hcross_corr) * ((calpha*calpha - salpha*salpha) - I * (2.0*calpha*salpha))

where the second factor on the right-hand side is exp(-2*I*alpha). But since alpha is constant and known from the beginning, this multiplication can be done when the values of hplus_corr and hcross_corr are originally being constructed. That, in turn, is done in these lines, which are essentially

(hplus_corr - I * hcross_corr) += (curr_h_real + I * curr_h_imag) * curr_ylm

Therefore, by distributivity and associativity, the multiplication by exp(-2*I*alpha) can be done instead by multiplying curr_ylm when its value is being set. Since that requires just one complex multiplication per (l, m) pair, it is much more efficient — while still consistent with the more precise viewpoint that spin-weighted functions are actually functions on the rotation group, which in this notation means also a function of alpha.

Note that there appears to be some conflict between the code and the LIGO document to which it refers regarding the sign of alpha. This commit just maintains the choice made in the function being altered here, which relies on XLALSimInspiralNRWaveformGetRotationAnglesFromH5File.

Edited by Ghost User

Merge request reports

Pipeline #26983 passed

Pipeline passed for 50b9f7b9 on michael.boyle:simplify-spin-rotation

Closed by Adam MercerAdam Mercer 5 years ago (Nov 7, 2019 10:57pm UTC)

Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Michael Boyle changed the description

    changed the description

  • Harald just pointed out to me that there's a newer document that is more relevant than the one I linked to above (and is referred to in a comment in the code): the NR Injection Infrastructure document, which contains an updated version of the previous document.

  • @sebastian-khan, @ian-harry, you two seem to have the most recent experience with this code, could you please have a look and say if you see any red flags?

    Also, does this modify reviewed code of the NR injection infrastructure? If that's the case, we should at least inform the original review team.

    Thanks! Frank

  • Hi @michael.boyle thanks for posting this, and sorry this took so long to get to me. I think I need to sign up for emails from certain labels ... Not sure how to do that yet though.

    As far as I can see the code change makes sense. A quick question though: Previously the code was not assuming that the input vectors would be 0s. Now it is assumed that the input vectors are zeroed out. Can someone verify that this is always the case?

    As this is reviewed code, this would need to be brought to the attention of Harald and Mark and Patricia (who do not seem to have accounts here ... Or maybe I don't have enough permissions to add people). I think that this could be reviewed with a simple sanity check. For a few thousand examples (or so), does the code produce identical output (within machine precision) before and after this patch?

    Thanks! Ian

  • Karl Wette added 1 deleted label

    added 1 deleted label

  • Any updates on this, or can it be closed?

  • Closing as there's been no response in three months.

  • closed

  • Ghost User changed the description

    changed the description

Please register or sign in to reply
Loading