ValueError during compute_MVBS

[lsetiawan]

Overview

ValueError was discovered while running the OOI_EK60_mooringtimeseries example notebook during a review of #824.

Troubleshooting

While my initial thought of this issue was caused by the new combine_echodata method, further investigation suggest that this issue started appearing in version 0.6.1!

• 0.6.0: Kernel died when trying to run through all cells up to compute_MVBS. Had to re-run from getting converted files, combining, then calculate Sv and MVBS. Other than that, everything ran through successfully.
• 0.6.1: Kernel died when trying to run through all cells up to compute_MVBS. Had to re-run from getting converted files, combining, then calculate Sv and MVBS. During the compute_MVBS I get

  ValueError: echo_range variable changes across pings in at least one of the frequency channels.

• 0.6.2: Everything ran smooth without any memory spikes up to compute_Sv with offload_to_zarr=True. When I got to compute_Sv it worked but was in borderline of my max memory capacity, running at about 98% with fully using all of my swap, but successfully get ds_Sv. During the compute_MVBS I get

  ValueError: echo_range variable changes across pings in at least one of the frequency channels.

Looking at git history, seems like the current code change to compute_MVBS happened in PR #753. The full error points to the line with this change: https://github.com/OSOceanAcoustics/echopype/pull/753/files#diff-2848209b968526da06d5475e3df089d5a7138b944681119fbe9f64f9aad81603R118.

I'm not sure why this issue isn't caught during CI testing, but seems like there's a current issue to rework the test for compute_MVBS see #756.

Full error

---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
Input In [34], in <cell line: 1>()
----> 1 ds_MVBS = ep.preprocess.compute_MVBS(
      2     ds_Sv, 
      3     range_meter_bin=0.2,  # 0.2 meters
      4     ping_time_bin='10S'   # 10 seconds
      5 )

File /opt/conda/envs/ep062/lib/python3.9/site-packages/echopype/preprocess/api.py:119, in compute_MVBS(ds_Sv, range_meter_bin, ping_time_bin)
     96 """
     97 Compute Mean Volume Backscattering Strength (MVBS)
     98 based on intervals of range (``echo_range``) and ``ping_time`` specified in physical units.
   (...)
    115 A dataset containing bin-averaged Sv
    116 """
    118 if not ds_Sv["echo_range"].groupby("channel").apply(_check_range_uniqueness).all():
--> 119     raise ValueError(
    120         "echo_range variable changes across pings in at least one of the frequency channels."
    121     )
    123 # Groupby freq in case of different echo_range (from different sampling intervals)
    124 range_interval = np.arange(0, ds_Sv["echo_range"].max() + range_meter_bin, range_meter_bin)

ValueError: echo_range variable changes across pings in at least one of the frequency channels.

[leewujung]

It seems that I had some bug in there... 😞

[leewujung]

This turns out to be due to something else all together:

In EchoData._harmonize_env_param_time() the interpolation from Environment/time1 to Sonar/Beam_group1/ping_time was done without considering NA that may exist in Environment/SOME_VAR. This can result in producing additional NA at the locations adjacent to the NA entry.
This interpolation was added in #755 to fix the problem from mismatched timestamp names in the Environment and Sonar/Beam_groupX groups.

The extra NAs causes _check_range_uniqueness to return False because the the echo_range contains an all-NA ping unexpectedly. In #837 I drop the NAs in Environment/SOME_VAR before the interpolation.

This also highlights the pretty urgent need for better tests for our processing code....

[leewujung]

Also relevant here is that preproces/api.py::_check_range_uniqueness is actually a limiting factor of the current compute_MVBS that should be removed, because changes in echo_range is expected and pretty common. See #662.

There is also efficiency issues in compute_MVBS at the moment if the data is delayed. For the 19 files in the OOI eclipse notebook, if I persist the ds_Sv data in memory it takes 5 mins to compute, but if ds_Sv is lazy-loaded it never finishes -- I wonder if there is some kind of double delayed situation going on, but haven't had a chance to look into that.

[leewujung]

Closing this as it has been addressed by #837. I'll open a new issue for improving the performance of compute_MVBS.