Skip to content

Fix parsing and printing of numpy types for numpy 2.0 #26112

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jan 24, 2024
Merged

Fix parsing and printing of numpy types for numpy 2.0 #26112

merged 1 commit into from
Jan 24, 2024

Conversation

oscarbenjamin
Copy link
Collaborator

References to other Issues or PRs

#26094

Brief description of what is fixed or changed

Changed conversion from numpy types to Float and printing of numpy types like np.PINF in preparation for numpy 2.0.

Other comments

Release Notes

  • core
    • Conversion from numpy types like float64 has been changed for compatibility with numpy 2.0.
  • printing
    • Code printers for numpy no longer use np.PINF or np.NINF which will be removed in numpy 2.0.

@sympy-bot
Copy link

sympy-bot commented Jan 23, 2024 •
edited
Loading

✅

Hi, I am the SymPy bot. I'm here to help you write a release notes entry. Please read the guide on how to write release notes.

Your release notes are in good order.

Here is what the release notes will look like:

  • core

    • Conversion from numpy types like float64 has been changed for compatibility with numpy 2.0. (#26112 by @oscarbenjamin)

  • printing

    • Code printers for numpy no longer use np.PINF or np.NINF which will be removed in numpy 2.0. (#26112 by @oscarbenjamin)

This will be added to https://github.com/sympy/sympy/wiki/Release-Notes-for-1.13.

Click here to see the pull request description that was parsed. 
<!-- Your title above should be a short description of what
was changed. Do not include the issue number in the title. -->

#### References to other Issues or PRs
<!-- If this pull request fixes an issue, write "Fixes #NNNN" in that exact
format, e.g. "Fixes #1234" (see
https://tinyurl.com/auto-closing for more information). Also, please
write a comment on that issue linking back to this pull request once it is
open. -->

https://github.com/sympy/sympy/issues/26094

#### Brief description of what is fixed or changed

Changed conversion from numpy types to Float and printing of numpy types like `np.PINF` in preparation for numpy 2.0.

#### Other comments


#### Release Notes

<!-- Write the release notes for this release below between the BEGIN and END
statements. The basic format is a bulleted list with the name of the subpackage
and the release note for this PR. For example:

* solvers
  * Added a new solver for logarithmic equations.

* functions
  * Fixed a bug with log of integers. Formerly, `log(-x)` incorrectly gave `-log(x)`.

* physics.units
  * Corrected a semantical error in the conversion between volt and statvolt which
    reported the volt as being larger than the statvolt.

or if no release note(s) should be included use:

NO ENTRY

See https://github.com/sympy/sympy/wiki/Writing-Release-Notes for more
information on how to write release notes. The bot will check your release
notes automatically to see if they are formatted correctly. -->

<!-- BEGIN RELEASE NOTES -->
* core
    * Conversion from numpy types like float64 has been changed for compatibility with numpy 2.0.
* printing
    * Code printers for numpy no longer use np.PINF or np.NINF which will be removed in numpy 2.0.
<!-- END RELEASE NOTES -->

Update

The release notes on the wiki have been updated.

asmeurer
asmeurer reviewed Jan 23, 2024
View reviewed changes
sympy/core/sympify.py
@@ -88,8 +90,8 @@ def _convert_numpy_types(a, **sympify_args):
prec = np.finfo(a).nmant + 1
Copy link
Member

Choose a reason for hiding this comment

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

Are we sure this logic is correct?

>>> a = np.float32(232132.2)
>>> a
232132.2
>>> sympify(a)
232132.
>>> Float('232132.2', precision=24)
232132.
>>> Float('232132.2', precision=25)
232132.2

Not completely sure why this happens. Some discrepancy between how a mpmath and machine float are implemented.

Copy link
Member

Choose a reason for hiding this comment

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

Or maybe this is an issue with sympy

>>> Float('232132.2', precision=24)._mpf_
(0, mpz(14856461), -6, 24)
>>> Float('232132.2', precision=25)._mpf_
(0, mpz(14856461), -6, 24)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't think that numpy's nmant is consistent: numpy/numpy#22333

Or maybe this is an issue with sympy

>>> Float('232132.2', precision=24)._mpf_
(0, mpz(14856461), -6, 24)
>>> Float('232132.2', precision=25)._mpf_
(0, mpz(14856461), -6, 24)

The _mpf_ is not expected to be affects by the precision argument.

Are we sure this logic is correct?

>>> a = np.float32(232132.2)
>>> a
232132.2
>>> sympify(a)
232132.
>>> Float('232132.2', precision=24)
232132.
>>> Float('232132.2', precision=25)
232132.2

This is to do with printing:

In [8]: a = np.float32(232132.2)

In [9]: sa = sympify(a)

In [10]: sa
Out[10]: 232132.

In [11]: sa._prec
Out[11]: 24

In [12]: Float(sa, precision=25)
Out[12]: 232132.2

@asmeurer
Copy link
Member

Not really related to this PR, but where is the logic that converts numpy arrays to ImmutableDenseNDimArray? It's confusing that it's not in the same place here.

@oscarbenjamin
Copy link
Collaborator Author

where is the logic that converts numpy arrays to ImmutableDenseNDimArray?

sympy/sympy/core/sympify.py

Lines 417 to 425 in e077db8

if not strict:
# Put numpy array conversion _before_ float/int, see
# <https://github.com/sympy/sympy/issues/13924>.
flat = getattr(a, "flat", None)
if flat is not None:
shape = getattr(a, "shape", None)
if shape is not None:
from sympy.tensor.array import Array
return Array(a.flat, a.shape) # works with e.g. NumPy arrays

@oscarbenjamin
Copy link
Collaborator Author

I don't see a simple way to disable the doctests in biomechanics.rst when matplotlib is not installed.

@asmeurer
Copy link
Member

asmeurer commented Jan 23, 2024 •
edited
Loading

I manually ran the test_optional_dependencies script locally with the git numpy. I got this failure, which seems legitimate

_____________________________________________________________________________ test_pole_zero ______________________________________________________________________________
sympy/physics/control/tests/test_control_plots.py:121: in test_pole_zero
    assert pz_tester(tf1, exp1)
E   assert False
E    +  where False = <function test_pole_zero.<locals>.pz_tester at 0x127f2a160>(TransferFunction(1, p**2 + 0.5*p + 2, p), [[], [(-0.24999999999999994+1.3919410907075054j), (-0.24999999999999994-1.3919410907075054j)]])

and didn't find any other errors.

I don't know where the "numpy nightly" CI builds came from, but I wouldn't worry about it too much. NumPy is going to release a release candidate for 2.0 in a week or so, and once that happens it will presumably be easier to get compatible versions of scipy, matplotlib, and so on.

@asmeurer
Copy link
Member

The changes here look good to me.

@oscarbenjamin
Copy link
Collaborator Author

Thanks

@oscarbenjamin oscarbenjamin merged commit b60bada into sympy:master Jan 24, 2024
@oscarbenjamin oscarbenjamin deleted the pr_numpy_2.0 branch January 24, 2024 00:54
This was referenced Jan 24, 2024
Closed
Merged
@oscarbenjamin oscarbenjamin mentioned this pull request Mar 26, 2024
Merged
@mikedh mikedh mentioned this pull request May 7, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@asmeurer asmeurer asmeurer left review comments

Assignees
No one assigned
Labels
core printing
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@oscarbenjamin @sympy-bot @asmeurer