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

Add variable transmit parameter support to BB pulse compression, fixes #925 #1293

Closed MohamedNasser8 closed 2 weeks ago

MohamedNasser8 commented 3 months ago

Overview

This PR introduces the ability to handle varying transmit parameters across pings in the BB pulse compression processing of the echopype library.

Changes Made

Benefits

This update allows for more comprehensive analysis of echosounder data, particularly in advanced operations where transmit parameters are not constant.

Additional Notes

Further optimization for memory usage and processing efficiency may be required as part of future work.

Example

Here's a snippet of the updated function demonstrating the handling of variable parameters:


# Iterate over each ping
for i in range(len(beam.ping_time)):
    # Select the parameters for the current ping
    current_params = {p: tx_params[p][i] for p in tx_param_names}
    # ... rest of the processing logic ...
MohamedNasser8 commented 3 months ago

Testing

The function has been tested with the test_calibrate_ek80 , confirming correct behavior for constant ping sizes. Further testing is required for varying ping sizes, and I am currently seeking datasets that exhibit this variability for comprehensive validation.

codecov-commenter commented 2 months ago

Codecov Report

Attention: Patch coverage is 42.85714% with 12 lines in your changes are missing coverage. Please review.

Project coverage is 88.08%. Comparing base (79bb587) to head (4d7a033). Report is 57 commits behind head on dev.

Files Patch % Lines
echopype/calibrate/ek80_complex.py 42.85% 12 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## dev #1293 +/- ## ========================================== + Coverage 83.52% 88.08% +4.55% ========================================== Files 64 11 -53 Lines 5686 990 -4696 ========================================== - Hits 4749 872 -3877 + Misses 937 118 -819 ``` | [Flag](https://app.codecov.io/gh/OSOceanAcoustics/echopype/pull/1293/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=OSOceanAcoustics) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/OSOceanAcoustics/echopype/pull/1293/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=OSOceanAcoustics) | `88.08% <42.85%> (+4.55%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=OSOceanAcoustics#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

MohamedNasser8 commented 2 months ago

Hello @leewujung, currently I don't have test data with varying parameters across pings for this PR. It's not yet covered by tests, so could you review the implementation?

leewujung commented 2 weeks ago

Closing because there are upstream changes that need to be included in this, and the pulse compression code has changed significantly since. Let's take this on another time and also address the optimization.