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

NEP 50 adoption related improvements/followups #24856

Open
1 of 7 tasks
seberg opened this issue Oct 4, 2023 · 5 comments
Open
1 of 7 tasks

NEP 50 adoption related improvements/followups #24856

seberg opened this issue Oct 4, 2023 · 5 comments
Assignees
Labels
62 - Python API Changes or additions to the Python API. Mailing list should usually be notified.
Milestone

Comments

@seberg
Copy link
Member

seberg commented Oct 4, 2023

Please don't hesitate to edit this issue to expand/clarify/add/remove items

Adopting NEP 50 is the most important part of a NumPy 2.0 release opinion because it consists of larger changes that we cannot possibly via a warning/deprecation path. IMO it is also by far the most important part of the idea of being as compatible as possible with the array api.

gh-23912 contains the maybe largest piece to try and adopt NEP 50 by default. In the PR, I listed the main things to be addressed/thought about. But an issue is better for tracking.

Note that I don't think I can find solutions and push on these all by myself. So help is needed. I also think that these issues have too many moving parts and it is probably impossible to wait for having a fix for all (or even most!) before we push on with removing the "legacy" mode!
Because of that, I really need others to chime in.

  • np.can_cast(123, "int8") currently assumes the default integer for the 123 which results in an unexpected result.
    • This is actually a tricky issue! Although we could special-case can_cast. (I am not sure if can_cast fixing is vital or not.
    • A similar thing happens for np.add(1., 5.3, dtype=np.intp) with cast checking (I do not consider this a big issue in practice.)
    • The larger fix, may require serious thoughts :(, such as implementing either:
      • Adding the notion of cast-safety to scalar assignment (setitem).
      • Adding abstract dtype instances for the abstract DTypes representing Python integers (mainly), which actually know the value and could define a cast safety. (But this adds a fair bit of complexity for a very specific issue!)
  • Vet as many functions as possible to see if they need to adapt to NEP 50 behavior. Anyone can start looking into these!. New infrastructure may help, but even type(x) in (int, float, complex) is probably an ok start in many cases:
    • The PR has to do some dance to make percentile and quantile work as expected.
    • Other examples which use asanyarray() but should behave ufunc-like w.r.t. to weak promotion:
      • isclose() (requires asanyarray() for indexing)
  • One big inconvenience is that NEP 50 says that we cast all integers to the other integer type in binary ufuncs, or the default integer in single input one (or if all integers).
    • The main issue/inconvenience is that uint8 > -1 currently does not work.
    • I suspect fixing this is just a lot harder if you have to juggle more of the compatibility layer.
    • One fix could be a family of comparison functions when python integers are involved (unfortunately, that is a lot of functions!)
  • It would be nice to replace many or even all of the current type resolvers
    • Would help with reducing complexity a lot.
    • Requires the iteration to disable legacy mode
  • As noted in Query: isnan/isinf and NEP50 #24712, some functions should work for integer inputs on the grounds that the result is boolean and trivial (e.g. also if we would cast to float). The initial PR ensures these work, but does so in a way which is not ideal.

The main points above are that e.g. can_cast logic may require some thoughts, which makes this quite tough. Which also means that I cannot address all of these at once, there is just too many moving parts without pushing forwards with the main changes and others helping out, e.g. with vetting python functions to see what patterns will help us align them with NEP 50.

Further issues, which are not as high priority and only indirectly related are that we will want to change some other things such as disabling promotion of strings with numbers.

@seberg seberg added the 62 - Python API Changes or additions to the Python API. Mailing list should usually be notified. label Oct 4, 2023
@seberg seberg added this to the 2.0.0 release milestone Oct 4, 2023
@seberg seberg pinned this issue Oct 4, 2023
@seberg
Copy link
Member Author

seberg commented Oct 12, 2023

The PR now includes an approach for dealing with out-of-bound integer comparisons in a completely generic way: #23912 (comment)

This adds a fair bit of complexity, but recovers everything that used to work and makes it much much faster as any slow casts (maybe even to object) paths can now be avoided.

@ngoldbaum
Copy link
Member

Just a note from the review of gh-23912 so we don't forget: we need better user-facing docs explaining the NEP 50 semantics. That came up there where some discussion about the semantics was added to basics.creation.rst, but we really need a whole section elsewhere describing the rules that can be linked to from the there instead of a single example. The text could probably be adapted from NEP 50.

@rgommers
Copy link
Member

@seberg would you mind updating this issue? I think the window for 2.0.0 is almost closed, so it may be worth opening a separate issue for anything that still needs doing before RC1.

@seberg
Copy link
Member Author

seberg commented Mar 13, 2024

I will push the milestone, at this point, I think the question is more about what is reported than what is plausible to do. I can't do a big audit of Python functions right now for 100% correct behavior, so I think those are basically bug-fixes then (whether we wait for 2.1 or have them in 2.0.x).

@seberg seberg modified the milestones: 2.0.0 release, 2.1.0 release Mar 13, 2024
@seberg seberg changed the title Path to NEP 50 adoption NEP 50 adoption related improvements/followups Mar 13, 2024
@rgommers
Copy link
Member

rgommers commented Apr 1, 2024

I'm unpinning this now, since it's not critical anymore and doesn't need to be seen by everyone visiting the issue tracker.

@rgommers rgommers unpinned this issue Apr 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
62 - Python API Changes or additions to the Python API. Mailing list should usually be notified.
Projects
Status: 🏗 In progress
Development

No branches or pull requests

3 participants