echopype icon indicating copy to clipboard operation
echopype copied to clipboard

harmonize_env_param_time fails with index 0 error

Open valschmidt opened this issue 1 year ago • 7 comments

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

valschmidt avatar Mar 21 '24 13:03 valschmidt

@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 avatar Mar 28 '24 15:03 leewujung

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

leewujung avatar Apr 01 '24 17:04 leewujung

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

valschmidt avatar Apr 02 '24 14:04 valschmidt

@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.

ctuguinay avatar Apr 05 '24 23:04 ctuguinay

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.

valschmidt avatar Apr 07 '24 17:04 valschmidt

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:

  • first resolve the bug so that at least the Sv values can be computed correctly for this type of data
  • at a later time we need to address the performance side of things -- the padded NaNs in the raw-converted data may create a lot of potentially wasteful computations (we can see if that is the case and how bad that is), and I wonder if dropna is properly delayed if the computations would only be on the entries that have actual values. This is likely more involved though, as we know from trying to scale other functions, hence my recommendation to separate the scaling into a separate issue.

leewujung avatar Apr 08 '24 00:04 leewujung

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

ctuguinay avatar Apr 08 '24 19:04 ctuguinay

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 :)

ctuguinay avatar Apr 13 '24 20:04 ctuguinay

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

valschmidt avatar Apr 15 '24 13:04 valschmidt

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

ctuguinay avatar Apr 24 '24 15:04 ctuguinay

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: @.***>

valschmidt avatar Apr 25 '24 17:04 valschmidt