Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
leewujung opened this issue Jun 18, 2024 · 10 comments
Closed

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

leewujung opened this issue Jun 18, 2024 · 10 comments
Labels
dependencies Anything related to dependencies
Milestone

Comments

@leewujung
Copy link
Member

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

@leewujung leewujung added the dependencies Anything related to dependencies label Jun 18, 2024
@leewujung leewujung added this to the v0.9.0 milestone Jun 18, 2024
@github-project-automation github-project-automation bot moved this to Todo in Echopype Jun 18, 2024
@ctuguinay
Copy link
Collaborator

ctuguinay commented Jun 25, 2024

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
Copy link
Member Author

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

@ctuguinay
Copy link
Collaborator

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
Copy link
Collaborator

ctuguinay commented Jun 26, 2024

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
Copy link
Collaborator

Problem described above is #1332

@ctuguinay
Copy link
Collaborator

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

@ctuguinay
Copy link
Collaborator

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

@github-project-automation github-project-automation bot moved this from Todo to Done in Echopype Jul 16, 2024
@markseery
Copy link

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(arr_dtype, 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
Copy link
Collaborator

ctuguinay commented Aug 3, 2024

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
Copy link
Collaborator

comment for pip installing from main github branch: #1370 (comment)

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Anything related to dependencies
Projects
Status: Done
Development

No branches or pull requests

3 participants