Skip to content

Add getter for NumPy ufunc types to patching#226

Open
ndgrigorian wants to merge 1 commit into
mainfrom
fix-for-numpy-2.5
Open

Add getter for NumPy ufunc types to patching#226
ndgrigorian wants to merge 1 commit into
mainfrom
fix-for-numpy-2.5

Conversation

@ndgrigorian

Copy link
Copy Markdown
Collaborator

NumPy 2.5 has changed how types property is exposed with ufunc.types. This caused mkl_umath to crash during patching on pre-release NumPy.

This PR adds a getter, _get_ufunc_types, defined in _patch_numpy.pyx as cdef extern. This getter works with NumPy 2.5 and previous versions by accessing the ufunc struct directly.

Additionally, slips in changes which prevent attempting free of nullptr when the patch fails (i.e., due to RuntimeError).

NumPy 2.5+ changed how types property is exposed in NumPy, causing indexing on the types to break

also fixes a bug where patching would attempt to free null pointers when patching process fails midway

@jharlow-intel jharlow-intel left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM!

@antonwolfy antonwolfy added this to the 0.5.0 release milestone Jun 12, 2026
free(self.functions)
if self.functions is not NULL:
for i in range(self.functions_count):
free(self.functions[i].signature)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It might be NULL when nargs == 0


self.functions = <function_info *> malloc(
self.functions_count * sizeof(function_info)
expected_count * sizeof(function_info)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Do we need to call malloc only when expected_count > 0?


cnp.import_umath()

cdef extern from *:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Need to populate the chnagelog

@@ -53,66 +63,74 @@ cdef class _patch_impl:
functions_dict = dict()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Should it be an instance attribute instead of class-level one?

    cdef dict functions_dict

In that case each _patch_impl instance owns its own mapping and it's freed with the instance (more robust approach).

But we need to add initialization to the cinit below:

    def __cinit__(self):
        self.functions_dict = {}
        ...

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.

3 participants