FEAT: add precessing spin transformation#1104
Conversation
GregoryAshton
left a comment
GregoryAshton
left a comment
There was a problem hiding this comment.
Broadly looks good to me - albeit the checks are failing
| return theta, phi | ||
|
|
||
|
|
||
| def transform_precessing_spins( |
There was a problem hiding this comment.
This is overwriting an existing function with a properly vectorised version if I have understood correctly. Typically, I'd think it would be better to use a new name. However, I can see there are tests to ensure the two agree.. so I'm inclined to say this seems like a good idea.
There was a problem hiding this comment.
The previous version was wrapped with np.vectorize, so the signature hasn't changed. Both the old and new ones accept vector inputs, the only difference is that these will now be faster because it uses array broadcasting instead of numpy.vectorize, which is just a for loop.
| - spin_2x, spin_2y, spin_2z: Components of spin 2 | ||
| """ | ||
|
|
||
| xp = array_module(theta_jn) |
There was a problem hiding this comment.
Does array_module here just take theta_jn to figure out the array type?
There was a problem hiding this comment.
This was using an outdated version. I updated it to use the xp_wrap decorator, which will look at all of the input arguments.
ColmTalbot
force-pushed
the
precessing-spin-transform
branch
from
c7918c2 to
da1d46f
Compare
ColmTalbot
force-pushed
the
precessing-spin-transform
branch
from
7a82a82 to
6b7f838
Compare
Updated tolerance for assert_allclose due to precision loss in different array backends.
Added error handling for ZeroDivisionError when calculating spins.
2 participants
This is a replacement for #1044 that includes the backend changes rather than attempting to rebase.