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

ENH: integrate: cumulative_simpson can support complex dtypes with small change #20553

Open
TheIdealis opened this issue Apr 22, 2024 · 4 comments
Labels
enhancement A new feature or improvement scipy.integrate

Comments

@TheIdealis
Copy link

Describe your issue.

If I use cumulative_simpson with a complex valued array, the imaginary part is discarded in _cumulatively_sum_simpson_integrals. The problem can be fixed easily by changing

sub_integrals = np.empty(shape)

to

sub_integrals = np.empty(shape, dtype=y.dtype)

Reproducing Code Example

from scipy.integrate import cumulative_simpson, simpson
import numpy as np

x = (1 + 1j) * np.ones(10)

print(cumulative_simpson(x))

Error message

.../python3.10/site-packages/scipy/integrate/_quadrature.py:857: ComplexWarning: Casting complex values to real discards the imaginary part
  sub_integrals[..., :-1:2] = sub_integrals_h1[..., ::2]

SciPy/NumPy/Python version and system information

cython
    version: 3.0.10
numpy
    version: 1.26.4
scipy
    version: 1.13.0
host:
    cpu: x86_64
    endian: little
    family: x86_64
    system: linux
Python Information:
  version: '3.10'
@TheIdealis TheIdealis added the defect A clear bug or issue that prevents SciPy from being installed or used as expected label Apr 22, 2024
@j-bowhay
Copy link
Member

cc @nprav

@lucascolley lucascolley changed the title BUG: cumulative_simpson is casting complex vaules to real ones BUG: integrate: cumulative_simpson is casting complex vaules to real ones Apr 22, 2024
@mdhaber mdhaber added enhancement A new feature or improvement and removed defect A clear bug or issue that prevents SciPy from being installed or used as expected labels Apr 22, 2024
@mdhaber mdhaber changed the title BUG: integrate: cumulative_simpson is casting complex vaules to real ones ENH: integrate: cumulative_simpson is casting complex vaules to real ones Apr 22, 2024
@mdhaber mdhaber changed the title ENH: integrate: cumulative_simpson is casting complex vaules to real ones ENH: integrate: cumulative_simpson can support complex dtypes with small change Apr 22, 2024
@mdhaber
Copy link
Contributor

mdhaber commented Apr 22, 2024

Typically, when the documentation doesn't specifically mention complex dtypes, it was developed with only real dtypes in mind. For instance, quad got support for complex valued functions relatively recently. The documentation of cumulative_simpson doesn't suggest that it should work for complex data, so we can consider this as an enhancement request.

@TheIdealis you're welcome to submit a PR. In addition to the adjustment you mentioned, please:

  • consider cumulative_trapezoid, simpson, trapezoid, and other similar functions that can easily support complex y
  • adjust the documentation to distinguish between real and complex inputs (e.g. the x input still needs to be real)
  • write a unit test shared between the functions to check for correct behavior

@TheIdealis
Copy link
Author

@mdhaber I'd like to do it, but I rarely ever did a PR so it might take some days to find the time for it. Just one small question. The documentation of e.g. cumulative_trapezoid is not specifying complex values either. Should I add this in those documentation entries as well? In the same manner as in e.g. fft: ", can be complex"?

@mdhaber
Copy link
Contributor

mdhaber commented Apr 22, 2024

Should I add this in those documentation entries as well?

Yes, that's what I mean by "consider cumulative_trapezoid, simpson, trapezoid, and other similar functions that can easily support complex y". I think we should ensure that similar functions have similar capabilities, and if we intend to provide those capabilities, they should be documented.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement A new feature or improvement scipy.integrate
Projects
None yet
Development

No branches or pull requests

4 participants
@TheIdealis @mdhaber @j-bowhay and others