search

OSOceanAcoustics / echopype

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

Handle missing `receiver_sampling_frequency` in EK80 data [all tests ci] #1219

Closed praneethratna closed 8 months ago

praneethratna commented 8 months ago

Addresses #1217 by adding a check for receiver_sampling_frequency not being equal to 0 or else setting it to its default value.

CC @leewujung

leewujung commented 8 months ago

@praneethratna : This looks great! I verified the file @icaro-00 and those that we already have in the test data, and confirmed that even if receiver_sampling_frequency exists it could be 0, which is a case we haven't seen previously. Before we have only seen files that do not contain this field altogether, from earlier versions of the EK80 software.

Please feel free to merge! Thanks!

leewujung commented 8 months ago

BTW, @icaro-00 : Do you have a file that is much smaller that we could include in our test data set (e.g., a few MB)? We typically do not include large files because that lengthens the time of our test runs significantly. Thanks!

icaro-00 commented 8 months ago

BTW, @icaro-00 : Do you have a file that is much smaller that we could include in our test data set (e.g., a few MB)? We typically do not include large files because that lengthens the time of our test runs significantly. Thanks!

Hi @leewujung, the smallest find I could find is 300MB. Here is the link: https://wetransfer.com/downloads/1e20766c3f67ddccda9cfbcecb20edec20231116101129/4d5f8f

Is the error fixed and the updated code available already? Is it going to be fixed with the 0.8.2 release?

Thank you!

icaro-00 commented 8 months ago

I actually managed to find another smaller file, 20MB.

Here the link: https://wetransfer.com/downloads/f54d5fd680e8d724926cd7849dfe782020231116120052/2d0d2b

leewujung commented 8 months ago

Thanks Vogt the files @icaro-00 !

You could try to pip install the dev branch that had the fix included, like shown here

Our goal is to get v0.8.2 out on Nov 20, so you could also wait for that if it’s not too far away for you.

leewujung commented 8 months ago

@praneethratna : I added the 20MB file into the google folder: ek80/D20230804-T083032.raw. Please add a test for testing for the stored receiver_sampling_frequency value when you have a chance. Thank you!

icaro-00 commented 8 months ago

Thanks Vogt the files @icaro-00 !

You could try to pip install the dev branch that had the fix included, like shown here

Our goal is to get v0.8.2 out on Nov 20, so you could also wait for that if it’s not too far away for you.

Thank you for the info @leewujung, I will wait for Nov 20, and thanks for helping on this!