Open RubendeBruin opened 8 months ago
Hi. Thanks for showing interest in our package.
With respect to your comment, have you seen my colleague's reply on this topic in your earlier conversation? https://github.com/4Subsea/waveresponse-python/issues/54#issuecomment-1628387093
we would welcome a PR, but we're a quite busy these days, so it might take a while before we have time to review
Hi Heidi, yes I have seen the comment. This report applies to the "polar" option already.
I'll prepare a PR, but note that I do not have access to your stories etc.
Hi, could you please provide more details about what you think is the issue? If you have a pr prepared already I can have a look.
Hi @helene-pisani-4ss , no I do not have a pr yet.
The issue is that the interpolation of the phase is incorrect. It should be something like sketch in green below:
A way to do this is: (ref https://github.com/RubendeBruin/mafredo/blob/master/src/mafredo/rao.py)
CU = np.exp(1j * data['phase'])
interpolate that complex unit. Note that this will decrease the amplitude, that is ok as we're only intested in the phase
Caluclate the phase from the interpolated CU:
phase = np.angle(CU)
For phase interpolation the correct method to use is "polar". In this method the phase and amplitude are interpolated independently.
However, phase jumps should be accounted for when interpolating. Testing with some dummy-data shows that this is currently not the case:
Original:
Interpolated:
A solution is unwrapping the phase data before interpolating. Because unwrapping in 2D is not trivial, this is easiest done in two steps. First interpolate in frequency, then in direction (of vice versa).
Would you welcome a PR implementing this?