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

[Bug]: AttributeError: module 'scipy.signal' has no attribute 'cwt' (deprecated, use PyWavelets instead) #912

Open
3 tasks
tomvothecoder opened this issue Jan 14, 2025 · 10 comments · May be fixed by #913
Open
3 tasks
Assignees
Labels
bug Bug fix (will increment patch version)

Comments

@tomvothecoder
Copy link
Collaborator

tomvothecoder commented Jan 14, 2025

What happened?

Per https://docs.scipy.org/doc/scipy-1.12.0/reference/generated/scipy.signal.cwt.html:

    .. deprecated:: 1.12.0

        scipy.signal.cwt is deprecated in SciPy 1.12 and will be removed
        in SciPy 1.15. We recommend using PyWavelets instead.

What did you expect to happen? Are there are possible answers you came across?

  • Add PyWavelets to dev.yml, ci.yml and pyproject.toml
  • Replace scipy.signal.cwt with PyWavelets
    • Tuple(np.ndarray, np.ndarray)
      The period and PSD arrays.
      """
      deg = 6
      period = np.arange(1, 55 + 1)
      freq = 1 / period
      widths = deg / (2 * np.pi * freq)
      cwtmatr = scipy.signal.cwt(data, scipy.signal.morlet2, widths=widths, w=deg)
      psd = np.mean(np.square(np.abs(cwtmatr)), axis=1)
      return (period, psd)
  • Update E3SM Unified notes that PyWavelets is a new dependency

Minimal Complete Verifiable Example (MVCE)

No response

Relevant log output

Traceback (most recent call last):
  File "/__w/e3sm_diags/e3sm_diags/e3sm_diags/parameter/core_parameter.py", line 340, in _run_diag
    single_result = module.run_diag(self)
  File "/__w/e3sm_diags/e3sm_diags/e3sm_diags/driver/qbo_driver.py", line 100, in run_diag
    test_dict["wave_period"], test_dict["wavelet"] = _calculate_wavelet(
  File "/__w/e3sm_diags/e3sm_diags/e3sm_diags/driver/qbo_driver.py", line 390, in _calculate_wavelet
    wave_period, wavelet = _get_psd_from_wavelet(detrended_data)
  File "/__w/e3sm_diags/e3sm_diags/e3sm_diags/driver/qbo_driver.py", line 418, in _get_psd_from_wavelet
    cwtmatr = scipy.signal.cwt(data, scipy.signal.morlet2, widths=widths, w=deg)
AttributeError: module 'scipy.signal' has no attribute 'cwt'. Did you mean: 'czt'?

Anything else we need to know?

Found in PR #907, where integration tests are breaking due to this issue.

Related SciPy docs:

Related PyWavelets docs:

Environment

Latest e3sm_diags on main
scipy=1.15.1

@tomvothecoder tomvothecoder added the bug Bug fix (will increment patch version) label Jan 14, 2025
@tomvothecoder tomvothecoder self-assigned this Jan 14, 2025
@xylar
Copy link
Contributor

xylar commented Jan 15, 2025

@tomvothecoder, @chengzhuzhang, @whannah1, I would prefer not to delay Unified testing for this. Could we consider constraining scipy<1.15 in e3sm_diags for the time being while you all work this out?

A second question: do you all have a strategy for detecting deprecation warnings like this (e.g. as outputs from your testing) so they can be addressed before they become a last-minute problem? I know tests spit out a lot of deprecation warnings and sometimes they're related to other tools (like xarray) but they can sometimes be a helpful indication that something like this is coming.

@forsyth2
Copy link
Collaborator

I would prefer not to delay Unified testing for this.

@chengzhuzhang @tomvothecoder In case you miss it on the zppy side, note my comments on E3SM-Project/zppy#634 (reply in thread) as well.

@tomvothecoder
Copy link
Collaborator Author

@tomvothecoder, @chengzhuzhang, @whannah1, I would prefer not to delay Unified testing for this. Could we consider constraining scipy<1.15 in e3sm_diags for the time being while you all work this out?

Yes, I agree with constraining scipy<1.15 (my comment just now).

A second question: do you all have a strategy for detecting deprecation warnings like this (e.g. as outputs from your testing) so they can be addressed before they become a last-minute problem? I know tests spit out a lot of deprecation warnings and sometimes they're related to other tools (like xarray) but they can sometimes be a helpful indication that something like this is coming.

The QBO wavelets feature is relatively new (I think around Oct/2024) and SciPy 1.15.0 was released early Jan/2025. The integration tests spits out deprecation warnings and I normally document them to address ahead of time. This one unfortunately slipped through the cracks somehow. We'll stay more on top of it moving forward.

@xylar
Copy link
Contributor

xylar commented Jan 15, 2025

Okay, thanks for the context. I just wouldn't want this kind of surprise to catch us on a regular basis.

@whannah1
Copy link

Just now seeing this - We've always been aware of this issue, and we've been comparing our wavelet analysis using both scipy and pywavelets. There is some sort of internal normalization going on that causes the two results to differ in magnitude slightly. This is why we didn't use pywavelets for the initial implementation of the QBO wavelet spectra.

However, in practice, this difference doesn't matter at all as long as all spectra are computed with the same package, so I don't see a problem updating things to use pywavelets whenever someone has time.

@forsyth2
Copy link
Collaborator

@tomvothecoder how should I proceed with zppy's release candidate?

  1. Manually change the e3sm_diags conda dev file with scipy < 1.15 and re-test zppy?
  2. Wait for you to merge a conda dev file update in e3sm_diags, then cherry-pick that commit to avoid the post-CDAT migration code, and then re-test zppy?
  3. Just update the expected results in zppy for now, and realize these will change when this issue is fixed during the RC testing period?

@chengzhuzhang
Copy link
Contributor

Just now seeing this - We've always been aware of this issue, and we've been comparing our wavelet analysis using both scipy and pywavelets. There is some sort of internal normalization going on that causes the two results to differ in magnitude slightly. This is why we didn't use pywavelets for the initial implementation of the QBO wavelet spectra.

However, in practice, this difference doesn't matter at all as long as all spectra are computed with the same package, so I don't see a problem updating things to use pywavelets whenever someone has time.

@whannah1 thank you for your insights! Good to know that both methods have been tested and using pywavelets would work just fine. I'm wondering if you could provide the equivalent pywavelets implementation codes, we will try to incorporate and test before finalizing e3sm_diags v3.

@tomvothecoder
Copy link
Collaborator Author

@tomvothecoder how should I proceed with zppy's release candidate?

1. Manually change the `e3sm_diags` conda dev file with `scipy < 1.15` and re-test `zppy`?

2. Wait for you to merge a conda dev file update in `e3sm_diags`, then cherry-pick that commit to avoid the post-CDAT migration code, and then re-test `zppy`?

3. Just update the expected results in `zppy` for now, and realize these will change when this issue is fixed during the RC testing period?

I think 1. is fine. We'll merge PR #907 with the constraint for scipy <1.15, then you can re-test with zppy. Once this issue is resolved (hopefully during rc testing period), we'll need to do 3.

@whannah1
Copy link

@whannah1 thank you for your insights! Good to know that both methods have been tested and using pywavelets would work just fine. I'm wondering if you could provide the equivalent pywavelets implementation codes, we will try to incorporate and test before finalizing e3sm_diags v3.

To be clear, we never found an "equivalent" implementation due to the magnitude issue I mentioned previously.

Here's some python routines I used when investigating the differences that might be helpful:

import scipy, pywt
#---------------------------------------------------------------------------------------------------
def get_psd_from_wavelet_scipy(data):
   deg = 6
   period = np.arange(1, longest_period + 1)
   freq = 1 / period
   widths = deg / (2 * np.pi * freq)
   cwtmatr = scipy.signal.cwt(data, scipy.signal.morlet2, widths=widths, w=deg)
   psd = np.mean( np.square( np.abs(cwtmatr) ), axis=1)
   return (period, psd)
#---------------------------------------------------------------------------------------------------
def get_psd_from_wavelet_pywt(data):
   deg = 6
   period = np.arange(1, longest_period + 1)
   widths = deg / ( 2 * np.pi / period )
   [cfs, freq] = pywt.cwt(data, scales=widths, wavelet='cmor1.5-1.0')
   psd = np.mean( np.square( np.abs(cfs) ), axis=1)
   period = 1 / freq
   return (period, psd)
#---------------------------------------------------------------------------------------------------

@chengzhuzhang
Copy link
Contributor

@whannah1 thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug fix (will increment patch version)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants