Fix regression bug in computing MVBS in selecting echo_range for specific frequency

[leewujung]

This PR fixes a few small things in related to compute_MVBS operation. Does not address #662 😕

Tasks

• fix error that use echo_range from the first channel for all channels: this is a bug from the process of converting the data to be aligned with dimension frequency to channel in v0.6.0
• re-orders the dimensions for echo_range in the AZFP Sv output to be consistent with data variable Sv
• fix typing error: input range_meter_bin currently listed as int, but should be general as float

[codecov-commenter]

Codecov Report

Merging #736 (b6205e5) into dev (a14c500) will increase coverage by 6.70%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##              dev     #736      +/-   ##
==========================================
+ Coverage   82.16%   88.87%   +6.70%     
==========================================
  Files          45        9      -36     
  Lines        4183      683    -3500     
==========================================
- Hits         3437      607    -2830     
+ Misses        746       76     -670     

Flag	Coverage Δ
unittests	88.87% <100.00%> (+6.70%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
echopype/calibrate/calibrate_azfp.py	96.00% <100.00%> (+0.08%)	⬆️
echopype/preprocess/api.py	74.60% <100.00%> (-17.71%)	⬇️
echopype/preprocess/__init__.py	0.00% <0.00%> (-100.00%)	⬇️
echopype/preprocess/noise_est.py	73.68% <0.00%> (-21.06%)	⬇️
echopype/calibrate/calibrate_base.py	85.26% <0.00%> (-2.11%)	⬇️
echopype/calibrate/calibrate_ek.py	93.40% <0.00%> (-0.74%)	⬇️
echopype/convert/api.py
echopype/echodata/widgets/utils.py
echopype/utils/io.py
... and 33 more

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

[leewujung]

Should include a typing error fix in the input range_meter_bin. It is currently listed as int, but should be general as float.

[b-reyes]

@leewujung I am wondering if this is the most appropriately place for this. This line is necessary because of how we create the EchoData object for AZFP, correct? If so, should we instead correct the order in set_groups_azfp? I am fine with leaving this as is, I just thought I would bring this up.

[leewujung]

Good point, I agree this is the best done in set_groups_azfp. In fact this was part of my test to see if we can do without .drop("channel") in this line:

echopype/echopype/preprocess/api.py

Line 79 in b6205e5

ds["echo_range"].isel(ping_time=0).squeeze().drop("channel").data,

Somehow that extra dimension/coordinate only shows up for AZFP data, and I was wondering if it had anything to do with the order of the dimension, but that wasn't the reason.

I'll create an issue so we remember to do this in set_groups_azfp instead of here.

[b-reyes]

Sounds good!

[leewujung]

Oh actually, now that I looked at this again, the order of dimension to be changed is for echo_range and not Sv (which came from backscatter_r), so I guess nothing has to change for set_groups_azfp, but it is the AZFP part of compute_range that needs to change.

[b-reyes]

I was having a little trouble tracking down where that happened. The AZFP part of compute_range was using parts of the EchoData object, so I assumed that we needed to make corrections in set_groups_azfp. After looking into this a little further, I found that in the EK else statement of compute_range we do range_meter = range_meter.transpose("channel", "ping_time", "range_sample"), so maybe the fix is as simple as adding that to the AZFP if statement block.

[leewujung]

Oh right, cool that you tracked it down! I'll link your comment to that issue... v0.6.2... so that we don't have to do this now. :P

[b-reyes]

Everything looks good to me.

[leewujung]

Thanks @b-reyes ! I'll merge now!