OSOceanAcoustics / echopype

Enabling interoperability and scalability in ocean sonar data analysis
https://echopype.readthedocs.io/
Apache License 2.0
89 stars 70 forks source link

harmonize_env_param_time fails with index 0 error #1287

Closed valschmidt closed 2 months ago

valschmidt commented 3 months ago

Hi,

When I attempt to compute_Sv: ''' ep.calibrate.compute_Sv(echodata[0],waveform_mode='BB',encode_mode='complex') '''

I am receiving an error: IndexError: index 0 is out of bounds for axis 0 with size 0, which is coming from the attempt to squeeze the environmental parameters object after dropping the time1 column in harmonize_env_param_time() (line 56).

I am new to echopype so I may be missing something obvious, or perhaps something is odd about our data files. Perhaps this view of the environment object will be instructive for someone with more knowledge:

Notification_Center

leewujung commented 3 months ago

@valschmidt : I wonder if a bug got accidentally introduced in a recent PR attempting to address some performance issue with harmonize_env_param_time. Could you send us a .raw file or a saved EchoData zarr/netcdf file?

leewujung commented 3 months ago

Hey @valschmidt : Pinging again to see if you can send us an example file. We cannot reproduce this error using our own datasets.

valschmidt commented 3 months ago

Sure!

Here’s a link to on: https://universitysystemnh-my.sharepoint.com/:i:/g/personal/vyk2_usnh_edu/EZZHUF_AKZdJsOvwtvKbi-gBrBx_wv1dZzUm5hEmru_tSw?e=tsOfls

On Apr 1, 2024, at 13:33, Wu-Jung Lee @.***> wrote:

Hey @valschmidt https://github.com/valschmidt : Pinging again to see if you can send us an example file. We cannot reproduce this error using our own datasets.

— Reply to this email directly, view it on GitHub https://github.com/OSOceanAcoustics/echopype/issues/1287#issuecomment-2030215509, or unsubscribe https://github.com/notifications/unsubscribe-auth/AASPJV7HMTM4NLPJV4W2653Y3GK6VAVCNFSM6AAAAABFBOGDOWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMZQGIYTKNJQHE. You are receiving this because you were mentioned.


Val Schmidt (he/him) Center for Coastal and Ocean Mapping / Joint Hydrographic Center University of New Hampshire Chase Ocean Engineering Lab 24 Colovos Road Durham, NH 03824 e: vschmidt [AT] ccom.unh.edu m: 614.286.3726

ctuguinay commented 3 months ago

@valschmidt Thanks for the example file! This was pretty fun to dig into.

The error stems from the transmit_frequency_start and transmit_frequency_end in ed["Sonar/Beam_group1"] having NaNs, which in turn messes up the calculation of the sound_absorption data variable, and that causes the error in harmonize_env_param_time. These NaNs originate from the 2 channels that make up transmit_frequency_start and transmit_frequency_end having inconsistent ping_time values, and this inconsistency originates from the EK80 parser/datagram itself. I created a Gist that expands on this a bit more with the file you sent us: https://gist.github.com/ctuguinay/202141a9c263d6e3d2e7a0e8ff748508

@leewujung Let me know what you both think of this. I think one way to fix this in order to run compute_Sv is by manually (post-.open_raw) filling in the NaN variables in transmit_frequency_start and transmit_frequency_end with the appropriate frequency values. As for why the EK80 parser/datagram has this ping_time inconsistency, I am not sure.

valschmidt commented 3 months ago

Oh! This is because we are using the multiplexed ping option of the wbt mini. That’s why ping times for the two channels are different. Before I forget, thank you so much for looking out to this. I really appreciate it.

leewujung commented 3 months ago

Ah I see, I think this is the same as or similar to #743. It's so long ago but I haven't been able to carve out time to do. :/

@ctuguinay : could you take a look at that issue as well? This is time for us to address this "bug" since sequential pinging sequences are more common now. I think we probably should to take a 2-stage approach to resolve this:

ctuguinay commented 3 months ago

@leewujung I agree with this approach and I'll look at Brandyn's issue.

ctuguinay commented 2 months ago

Hey @valschmidt, could we use the example raw file you sent above for our testing suite? I would like to add it to an integration test in #1302. It is a very nice example of broadband complex multiplex EK80 data :)

valschmidt commented 2 months ago

YES!

On Apr 13, 2024, at 16:36, Caesar Tuguinay @.***> wrote:

Hey @valschmidt https://github.com/valschmidt, could we use the example raw file you sent above for our testing suite? I would like to add it to an integration test in #1302 https://github.com/OSOceanAcoustics/echopype/pull/1302. It is a very nice example of broadband complex multiplex EK80 data :)

— Reply to this email directly, view it on GitHub https://github.com/OSOceanAcoustics/echopype/issues/1287#issuecomment-2053755813, or unsubscribe https://github.com/notifications/unsubscribe-auth/AASPJV2BWKGNJYHBHNEAJVTY5GJNPAVCNFSM6AAAAABFBOGDOWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANJTG42TKOBRGM. You are receiving this because you were mentioned.


Val Schmidt (he/him) Center for Coastal and Ocean Mapping / Joint Hydrographic Center University of New Hampshire Chase Ocean Engineering Lab 24 Colovos Road Durham, NH 03824 e: vschmidt [AT] ccom.unh.edu m: 614.286.3726

ctuguinay commented 2 months ago

@valschmidt This should be taken care of with the merging of #1302. I'll be closing this issue now

valschmidt commented 2 months ago

Great! I’ll check it out!Sent from my iPhoneOn Apr 24, 2024, at 11:42, Caesar Tuguinay @.***> wrote: @valschmidt This should be taken care of with the merging of #1302. I'll be closing this issue now

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: @.***>