Skip to content

Conversation

WarrenWeckesser
Copy link
Member

Closes gh-16170.

@WarrenWeckesser WarrenWeckesser added scipy.integrate C/C++ Items related to the internal C/C++ code base labels May 26, 2024
@WarrenWeckesser WarrenWeckesser requested a review from steppi as a code owner May 26, 2024 21:27
@github-actions github-actions bot added the defect A clear bug or issue that prevents SciPy from being installed or used as expected label May 26, 2024
@WarrenWeckesser
Copy link
Member Author

The failed Windows job is not related to this PR. It looks like some sort of CI infrastructure issue:

[...]
 Downloading markdown_it_py-3.0.0-py3-none-any.whl (87 kB)
   ---------------------------------------- 87.5/87.5 kB 2.5 MB/s eta 0:00:00
Downloading pygments-2.18.0-py3-none-any.whl (1.2 MB)
   ---------------------------------------- 1.2/1.2 MB 37.4 MB/s eta 0:00:00
Downloading urllib3-2.2.1-py3-none-any.whl (121 kB)
   ---------------------------------------- 121.1/121.1 kB 3.6 MB/s eta 0:00:00
Downloading zipp-3.19.0-py3-none-any.whl (8.3 kB)
Downloading mdurl-0.1.2-py3-none-any.whl (10.0 kB)
ERROR: THESE PACKAGES DO NOT MATCH THE HASHES FROM THE REQUIREMENTS FILE. If you have updated the package versions, please update the hashes. Otherwise, examine the package contents carefully; someone may have tampered with them.
    unknown package:
        Expected sha256 7a89180babf3b6af321f5d6970f8a90d687977565a48f05dc4ce75dfb94c777d
             Got        52c33cdda6efdc34dc995f43c7b0cd170e99b63331e9f2bc1d7a2aad6471df30

Error: Process completed with exit code 1.

Copy link
Member

@rgommers rgommers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Warren. This change makes sense in an ideal world. I suspect however that this is a breaking change when users supply a function written in Cython (xref gh-20196).

To get around that, it may be needed to call the function once with a read-only array, catch ValueError/TypeError, and avoid using read-only arrays from then on if an error got caught.

@WarrenWeckesser
Copy link
Member Author

@rgommers, do you have an example of how this could cause a problem? Clearing the WRITEABLE bit dynamically has nothing to do with const-ness of arguments in compiled code.

@rgommers
Copy link
Member

Clearing the WRITEABLE bit dynamically has nothing to do with const-ness of arguments in compiled code.

You'd think that yes, based on how things should work - but tell that to Cython. The example in gh-17172 does what you say here, and it was failing for a very basic linalg.expm call. Cython chokes on read-only arrays unless arguments in the Cython function are declared as const.

@WarrenWeckesser
Copy link
Member Author

Heh, I even commented in gh-17172 (a mind like a steel sieve). OK, I'm not going to pursue this further.

If odeint was a new function, we could specify up front that the arrays passed to user functions will be read-only, so if a user implements their functions in Cython, they must use const appropriately. But I guess we can't add this now without introducing a backwards compatibility break. Maybe it could be considered for a SciPy 2.0 API change.

@rgommers
Copy link
Member

Maybe it could be considered for a SciPy 2.0 API change.

Agreed, and even then it's a stretch perhaps. At a minimum we first need to fix all our own Cython code to work with read-only arrays, and properly test that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C/C++ Items related to the internal C/C++ code base defect A clear bug or issue that prevents SciPy from being installed or used as expected scipy.integrate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: scipy.integrate.odeint returns wrong values when derivative function overwrites y
2 participants