Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dual profile naming scheme and running (dolfyn.adp) cleaning functions #373

Open
willcoxe opened this issue Jan 24, 2025 · 5 comments
Open
Assignees

Comments

@willcoxe
Copy link

Hello,

I am using the dual profile option in dolfyn to read an ad2cp file and then attempting to perform some basic operations according to
the adcp example. For dual profiles sda and sdb, sda functions perfectly but because the names are appended with an _avg in sdb, the second profile, many of the operations do not run correctly, such as rotating the frame and/or cleaning data based on pressure & on correlation. Also in the second profile the dsb.attrs["cell_size"] is missing which leads to issues with some of the operations described in the example.

May of these work once the _avg is removed from the name and the time dimension is adjusted to dsb["time"] instead of dsb["time_avg"] (e.g. dsb = dsb.swap_dims({'time_avg': 'time'}) ) and I make sure to manually set dsb.attrs["cell_size"].

One exception (there might be others), which I have not yet fully debugged is api.clean.remove_surface_interference(dsb).

>>> api.clean.remove_surface_interference(dsb)
Traceback (most recent call last):
  File "~/code/python/venvs/adcp2/lib/python3.13/site-packages/mhkit/dolfyn/adp/clean.py", line 397, in remove_surface_interference
    a[..., bds] = val
    ~^^^^^^^^^^
IndexError: too many indices for array: array is 1-dimensional, but 2 were indexed

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "myscript.py", line 95, in <module>
    dsb = api.clean.remove_surface_interference(dsb)
  File "~/code/python/venvs/adcp2/lib/python3.13/site-packages/mhkit/dolfyn/adp/clean.py", line 399, in remove_surface_interference
    a[..., bds] = 0
    ~^^^^^^^^^^
IndexError: too many indices for array: array is 1-dimensional, but 2 were indexed

I am guessing that others may run into this also when using dual profiles with Nortek instruments. I will continue to try to debug the problem and would appreciate any hints you might have.

@willcoxe
Copy link
Author

It was 'range'. I have added dsb = dsb.swap_dims({'range_avg': 'range'}) and it works now.

Might still be an idea to somehow catch this in the code somewhere.

@ssolson
Copy link
Contributor

ssolson commented Jan 28, 2025

Thanks @willcoxe for letting us know your experience.

@jmcvey3 do you have any thoughts on improving the user experience around this issue?

@jmcvey3 jmcvey3 self-assigned this Feb 4, 2025
@ssolson ssolson mentioned this issue Feb 4, 2025
@jmcvey3
Copy link
Contributor

jmcvey3 commented Feb 6, 2025

@willcoxe Apologies for the late reply. We don't have many test cases for dual-profiling instruments, hence there are more issues on that front.
Splitting the two profiles apart into two distinct datasets should solve this problem and is probably the easiest way to do so - I'll start looking into adding code to this respect. As you've discovered, the "time" and "range" coordinates are hard-coded into some of the basic functionality, so any variables tagged as "_avg" would have this tag removed in the second dataset.

@willcoxe
Copy link
Author

willcoxe commented Feb 6, 2025

No rush at all. Renaming and swapping dimensions worked for me for the moment, even though it's not the most elegant solution.

@jmcvey3
Copy link
Contributor

jmcvey3 commented Feb 6, 2025

@willcoxe The cleaning functions should be fixed in the above PR.
Which functions were failing because of the missing "time" variable? The rotate2 function appears to be working properly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants