Skip to content

fix: make awkward compatible with numexpr 2.11#3533

Merged
ianna merged 10 commits intomainfrom
ikrommyd/numpexpr-2.11-compatibility
Jun 13, 2025
Merged

fix: make awkward compatible with numexpr 2.11#3533
ianna merged 10 commits intomainfrom
ikrommyd/numpexpr-2.11-compatibility

Conversation

@ikrommyd
Copy link
Copy Markdown
Collaborator

No description provided.

@ikrommyd ikrommyd requested review from Copilot, ianna and pfackeldey and removed request for Copilot June 11, 2025 10:31
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds compatibility for NumExpr 2.11 by introducing version‐dependent cache initialization and routing through new .l and .c attributes.

  • Add a parse_version check for NumExpr ≥2.11 to create per‐version sub‐caches on necompiler internals.
  • Route all lookups through _numexpr_last, _names_cache, and _numexpr_cache locals.
  • Mirror the same branching logic in both evaluate and re_evaluate.
Comments suppressed due to low confidence (3)

src/awkward/_connect/numexpr.py:81

  • [nitpick] Using single‐letter attribute names like .l and .c is cryptic. Consider using more descriptive names or adding comments to clarify their purpose.
if not hasattr(numexpr.necompiler._numexpr_last, "l"):

src/awkward/_connect/numexpr.py:80

  • [nitpick] Add dedicated tests for the NumExpr ≥2.11.0 code paths to ensure new cache attributes (.l, .c) behave correctly and to prevent regressions.
if parse_version(numexpr.__version__) >= parse_version("2.11.0"):

src/awkward/_connect/numexpr.py:167

  • The error message reads awkwardly. Consider changing it to: "No previous evaluate() execution found." for clearer grammar.
"not a previous evaluate() execution found"

@ikrommyd
Copy link
Copy Markdown
Collaborator Author

@ianna @pfackeldey I'm not particularly familiar with numexpr so this is just my first attempt. Do your thing if you have better ideas. This "works" though although I don't know if I'm missing something with numexpr's free-threading support in version 2.11

@ikrommyd
Copy link
Copy Markdown
Collaborator Author

Oh and here the ci fails because of the problems #3531 is fixing haha

@pfackeldey
Copy link
Copy Markdown
Collaborator

@ikrommyd I'd rather have someone else than me have a look at this. I'm not at all familiar with numexpr.

@ikrommyd
Copy link
Copy Markdown
Collaborator Author

ikrommyd commented Jun 11, 2025

@ikrommyd I'd rather have someone else than me have a look at this. I'm not at all familiar with numexpr.

Yeah I tried to mimic how numexpr implements their own evaluate function by reading their code. I don't know anything more than you I'm afraid.
I opened a relevant issue for help: pydata/numexpr#514

@ikrommyd ikrommyd marked this pull request as ready for review June 12, 2025 16:05
@ikrommyd ikrommyd requested a review from henryiii June 12, 2025 16:33
@ikrommyd
Copy link
Copy Markdown
Collaborator Author

@henryiii @ianna I'm using importlib now and also a _numexpr_version_ge_2_11_0 global variable so that the version is not checked every single time

@ikrommyd
Copy link
Copy Markdown
Collaborator Author

The only test affected by this PR is tests/test_0119_numexpr_and_broadcast_arrays.py. Everything else is numpy 2.3

@ikrommyd
Copy link
Copy Markdown
Collaborator Author

For reference, it seems to work fine on free-threaded python.

iPython 3.13.3 experimental free-threading build (main, Apr  9 2025, 03:48:13) [Clang 20.1.0 ]
Type 'copyright', 'credits' or 'license' for more information
IPython 9.3.0 -- An enhanced Interactive Python. Type '?' for help.
Tip: You can use Ctrl-O to force a new line in terminal IPython

In [1]: import awkward as ak

In [2]: a = ak.Array([[1,2,3], [], [4,5]])

In [3]: b = ak.Array([1,2,3])

In [4]: ak._connect.numexpr.evaluate("a + b")
Out[4]: <Array [[2, 3, 4], [], [7, 8]] type='3 * var * int64'>

In [5]: ak._connect.numexpr.re_evaluate()
Out[5]: <Array [[2, 3, 4], [], [7, 8]] type='3 * var * int64'>

@ikrommyd
Copy link
Copy Markdown
Collaborator Author

image

Co-authored-by: Henry Schreiner <henry.fredrick.schreiner@cern.ch>
Copy link
Copy Markdown
Member

@ianna ianna left a comment

Choose a reason for hiding this comment

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

@ikrommyd - thanks! I don't know if you want to commit the Copilot suggestions.

@ikrommyd
Copy link
Copy Markdown
Collaborator Author

@ikrommyd - thanks! I don't know if you want to commit the Copilot suggestions.

Well I did the first suggestion which was to cache the parse_version result. The others are not fully complete and I'd rather do exactly what numexpr does here because we're using internal code. I'd rather use internal code exactly like they do because it's easier to track if they make changes. I think I'm happy with how it is right now.

Copy link
Copy Markdown
Member

@ianna ianna left a comment

Choose a reason for hiding this comment

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

@ikrommyd - Thanks for fixing this! Current implementation looks good to me.

@ianna ianna removed the request for review from pfackeldey June 13, 2025 07:13
@ianna ianna merged commit f672219 into main Jun 13, 2025
42 checks passed
@ianna ianna deleted the ikrommyd/numpexpr-2.11-compatibility branch June 13, 2025 07:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants