OSOceanAcoustics / echopype

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

Fix problems associated with `np.complex_` removal at numpy 2.0 #1342

Closed leewujung closed 2 months ago

leewujung commented 3 months ago

Error when checking subdtype at parse_base

AttributeError: `np.complex_` was removed in the NumPy 2.0 release. Use `np.complex128` instead.

Here's is one instance of the error message: https://github.com/OSOceanAcoustics/echopype/actions/runs/9558726670/job/26347885285#step:12:17945

ctuguinay commented 3 months ago

With the merging of #1315, numpy was pinned to numpy<2. The CI tests for the PR all pass but there are two CI processing level tests for the merge that failed even with this pinning. These tests failing may be related to this pinning. Not quite sure yet...

[gw1] [ 98%] FAILED echopype/tests/utils/test_processinglevels_integration.py::test_raw_to_mvbs[EK60-EK60-raw_and_xml_paths0-extras0] 
echopype/tests/utils/test_processinglevels_integration.py::test_raw_to_mvbs[AZFP-AZFP-raw_and_xml_paths1-extras1] 
[gw1] [ 98%] FAILED echopype/tests/utils/test_processinglevels_integration.py::test_raw_to_mvbs[AZFP-AZFP-raw_and_xml_paths1-extras1] 
echopype/tests/utils/test_source_filenames.py::test_scalars

I'll take a quick look at this tomorrow.

leewujung commented 3 months ago

This is curious. This is tengentially related to #1343.

ctuguinay commented 3 months ago

Ok, it seems to be a problem with the remove_background_noise function and more specifically the reindex operation in estimate_background_noise. I'm still trying to make sense of it.

ctuguinay commented 3 months ago

Ok so the problem happens here:

spreading_loss = 20 * np.log10(ds_Sv["echo_range"].where(ds_Sv["echo_range"] >= 1, other=1))
absorption_loss = 2 * ds_Sv["sound_absorption"] * ds_Sv["echo_range"]

# Compute power binned averages
power_cal = _log2lin(ds_Sv["Sv"] - spreading_loss - absorption_loss)
power_cal_binned_avg = 10 * np.log10(
    power_cal.coarsen(
        ping_time=ping_num,
        range_sample=range_sample_num,
        boundary="pad",
    ).mean()
)

# Compute noise
noise = power_cal_binned_avg.min(dim="range_sample", skipna=True)

# Align noise `ping_time` to the first index of each coarsened `ping_time` bin
noise = noise.assign_coords(ping_time=ping_num * np.arange(len(noise["ping_time"])))

# Limit max noise level
noise = noise.where(noise < noise_max, noise_max) if noise_max is not None else noise

# Upsample noise to original ping time dimension
Sv_noise = (
    noise.reindex({"ping_time": power_cal["ping_time"]}, method="ffill")
    + spreading_loss
    + absorption_loss
)

When printing out the ping time indices for noise after the reassignment of coordinates I get this:

image

And when printing out the original ping time indices for power_cal I get this:

image

That being the case, the reindex errors out since there's a comparison between integers and datetime objects being doing within that function.

The reason that the remove background noise test passes is because the ping time index for the mock Sv DataArray is consisting of integers and not datetime objects:

def test_remove_background_noise():
    """Test remove_background_noise on toy data"""

    # Parameters for fake data
    nchan, npings, nrange_samples = 1, 10, 100
    chan = np.arange(nchan).astype(str)
    ping_index = np.arange(npings)
    range_sample = np.arange(nrange_samples)
    data = np.ones(nrange_samples)

    # Insert noise points
    np.put(data, 30, -30)
    np.put(data, 60, -30)
    # Add more pings
    data = np.array([data] * npings)
    # Make DataArray
    Sv = xr.DataArray(
        [data],
        coords=[
            ('channel', chan),
            ('ping_time', ping_index),
            ('range_sample', range_sample),
        ],
    )
ctuguinay commented 3 months ago

Problem described above is https://github.com/OSOceanAcoustics/echopype/issues/1332

ctuguinay commented 2 months ago

Related to #1353, we should only be using complex64 (at least for EK)

ctuguinay commented 2 months ago

This was fixed via merging of #1350. No other calls of np.complex_ exist in the main branch code

markseery commented 2 months ago

I am still getting this problem on open_raw:

AttributeError: np.complex_ was removed in the NumPy 2.0 release. Use np.complex128 instead

ed = ep.open_raw( File "/Users/markseery/.pyenv/versions/3.9.19/lib/python3.9/site-packages/echopype/utils/prov.py", line 237, in inner dataobj = func(*args, **kwargs) File "/Users/markseery/.pyenv/versions/3.9.19/lib/python3.9/site-packages/echopype/convert/api.py", line 430, in open_raw parser.rectangularize_data( File "/Users/markseery/.pyenv/versions/3.9.19/lib/python3.9/site-packages/echopype/convert/parse_base.py", line 172, in rectangularize_data self._parse_and_pad_datagram( File "/Users/markseery/.pyenv/versions/3.9.19/lib/python3.9/site-packages/echopype/convert/parse_base.py", line 274, in _parse_and_pad_datagram padded_arr = self.pad_shorter_ping(arr_list) File "/Users/markseery/.pyenv/versions/3.9.19/lib/python3.9/site-packages/echopype/convert/parse_base.py", line 619, in pad_shorter_ping if np.issubdtype(arrdtype, np.complex): File "/Users/markseery/.pyenv/versions/3.9.19/lib/python3.9/site-packages/numpy/init.py", line 397, in getattr raise AttributeError( AttributeError: np.complex_ was removed in the NumPy 2.0 release. Use np.complex128 instead.

ctuguinay commented 2 months ago

The latest branch of Echopype on Github doesn't have this np.complex check anymore. I think pip installing from the latest branch should work. Pip installing from 0.8.4 which is the default distribution on PyPi will not work since it still has this np.complex check. We're also planning on releasing 0.9.0 to PyPi soon with this fix (and a lot of other changes).

ctuguinay commented 2 months ago

comment for pip installing from main github branch: https://github.com/OSOceanAcoustics/echopype/issues/1370#issuecomment-2265787160

the echopype conda-forge distribution will also be updated soon to have this fix