Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions mkl_fft/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,17 @@
rfft2,
rfftn,
)
from ._patch_numpy import (
is_patched,
mkl_fft,
patch_numpy_fft,
restore_numpy_fft,
)
from ._version import __version__

import mkl_fft.interfaces # isort: skip


__all__ = [
"fft",
"ifft",
Expand All @@ -57,6 +64,10 @@
"rfftn",
"irfftn",
"interfaces",
"mkl_fft",
"patch_numpy_fft",
"restore_numpy_fft",
"is_patched",
]

del _init_helper
124 changes: 124 additions & 0 deletions mkl_fft/_patch_numpy.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
#!/usr/bin/env python
# Copyright (c) 2017, Intel Corporation
#
# Redistribution and use in source and binary forms, with or without
# modification, are permitted provided that the following conditions are met:
#
# * Redistributions of source code must retain the above copyright notice,
# this list of conditions and the following disclaimer.
# * Redistributions in binary form must reproduce the above copyright
# notice, this list of conditions and the following disclaimer in the
# documentation and/or other materials provided with the distribution.
# * Neither the name of Intel Corporation nor the names of its contributors
# may be used to endorse or promote products derived from this software
# without specific prior written permission.
#
# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
# DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE
# FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
# SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
# CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

"""Define functions for patching NumPy with MKL-based NumPy interface."""

from contextlib import ContextDecorator
from threading import local as threading_local

import numpy as np

import mkl_fft.interfaces.numpy_fft as _nfft

_tls = threading_local()


class _Patch:
"""Internal object for patching NumPy with mkl_fft interfaces."""

_is_patched = False
__patched_functions__ = _nfft.__all__
_restore_dict = {}
Comment on lines +42 to +44
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

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

Using class attributes instead of instance attributes is a code smell. While each thread gets its own _Patch instance via thread-local storage, using class attributes like _is_patched and _restore_dict suggests they're shared across instances, which is misleading. These should be initialized in an init method as instance attributes (self._is_patched = False, self._restore_dict = {}, self.patched_functions = _nfft.all) to make the code clearer and avoid potential issues if the class is ever used outside of TLS.

Suggested change
_is_patched = False
__patched_functions__ = _nfft.__all__
_restore_dict = {}
def __init__(self):
# Per-instance state for patching; one _Patch instance is stored per-thread.
self._is_patched = False
self.__patched_functions__ = _nfft.__all__
self._restore_dict = {}

Copilot uses AI. Check for mistakes.

def _register_func(self, name, func):
if name not in self.__patched_functions__:
raise ValueError("%s not an mkl_fft function." % name)
f = getattr(np.fft, name)
self._restore_dict[name] = f
setattr(np.fft, name, func)

def _restore_func(self, name, verbose=False):
if name not in self.__patched_functions__:
raise ValueError("%s not an mkl_fft function." % name)
try:
val = self._restore_dict[name]
except KeyError:
if verbose:
print("failed to restore")
return
else:
if verbose:
print("found and restoring...")
setattr(np.fft, name, val)

def restore(self, verbose=False):
for name in self._restore_dict.keys():
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

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

The restore method iterates over _restore_dict.keys() instead of the original patched_functions list. This means if do_patch() was interrupted partway through (e.g., due to an exception), only the functions that were successfully patched will be restored. While this might be the intended behavior, it could leave numpy in an inconsistent state. Consider whether this is the desired behavior or if all functions should be attempted for restoration.

Suggested change
for name in self._restore_dict.keys():
# Attempt to restore all functions that are part of the patching contract.
# _restore_func will safely handle cases where no original value was recorded.
for name in self.__patched_functions__:

Copilot uses AI. Check for mistakes.
self._restore_func(name, verbose=verbose)
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

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

After restoring all functions, the _restore_dict should be cleared to prevent memory leaks and allow for proper re-patching. Add "self._restore_dict.clear()" after the loop on line 69.

Suggested change
self._restore_func(name, verbose=verbose)
self._restore_func(name, verbose=verbose)
self._restore_dict.clear()

Copilot uses AI. Check for mistakes.
self._is_patched = False

def do_patch(self):
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

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

Calling do_patch() multiple times without calling restore() in between will overwrite _restore_dict values, losing the original NumPy functions. This prevents proper restoration. Add a check: if self._is_patched is True, either skip re-patching or raise an error to prevent this issue.

Suggested change
def do_patch(self):
def do_patch(self):
# Avoid re-patching when already patched, which would overwrite
# the originals stored in _restore_dict with the patched versions.
if self._is_patched:
return

Copilot uses AI. Check for mistakes.
for f in self.__patched_functions__:
self._register_func(f, getattr(_nfft, f))
self._is_patched = True

def is_patched(self):
return self._is_patched


def _initialize_tls():
_tls.patch = _Patch()
_tls.initialized = True


def _is_tls_initialized():
return (getattr(_tls, "initialized", None) is not None) and (
_tls.initialized is True
)
Comment on lines +87 to +89
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

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

The condition check is redundant. The expression "(getattr(_tls, 'initialized', None) is not None) and (_tls.initialized is True)" can be simplified to just "getattr(_tls, 'initialized', False)" since the attribute is always set to True when initialized. The current check is unnecessarily complex.

Suggested change
return (getattr(_tls, "initialized", None) is not None) and (
_tls.initialized is True
)
return getattr(_tls, "initialized", False)

Copilot uses AI. Check for mistakes.


def patch_numpy_fft(verbose=False):
if verbose:
print("Now patching NumPy FFT submodule with mkl_fft NumPy interface.")
print(
"Please direct bug reports to https://github.com/IntelPython/mkl_fft"
)
if not _is_tls_initialized():
_initialize_tls()
_tls.patch.do_patch()
Comment on lines 92 to 100
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

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

Using thread-local storage for tracking patch state creates a mismatch between local tracking and global effects. When one thread calls patch_numpy_fft(), it modifies the global np.fft namespace (affecting all threads), but only sets _is_patched=True in its own thread-local storage. This means: 1) Thread B won't see that Thread A has patched, and might try to patch again (triggering the double-patch issue), and 2) Thread A might restore while Thread B still needs the patch. Consider using a global lock and counter to properly coordinate multi-threaded patching, or document that this is not thread-safe for concurrent patch/restore calls.

Copilot uses AI. Check for mistakes.


def restore_numpy_fft(verbose=False):
if verbose:
print("Now restoring original NumPy FFT submodule.")
if not _is_tls_initialized():
_initialize_tls()
_tls.patch.restore(verbose=verbose)


def is_patched():
if not _is_tls_initialized():
_initialize_tls()
return _tls.patch.is_patched()


class mkl_fft(ContextDecorator):
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

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

The class name "mkl_fft" conflicts with the module name "mkl_fft". This creates ambiguity: when users import mkl_fft, they get the module, but mkl_fft.init.py also exports a class called mkl_fft. This makes it impossible to use the context manager class directly without confusion. Consider renaming the class to something like "patch_context" or "numpy_fft_context" to avoid this naming collision.

Copilot uses AI. Check for mistakes.
def __enter__(self):
patch_numpy_fft()
return self

def __exit__(self, *exc):
restore_numpy_fft()
return False
Comment on lines 92 to 124
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

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

The new patching functionality lacks documentation. Consider adding: 1) docstrings to all public functions (patch_numpy_fft, restore_numpy_fft, is_patched), 2) docstring to the context manager class explaining its usage, 3) updating README.md to explain this new feature and provide usage examples, and 4) adding a note about thread-safety considerations.

Copilot uses AI. Check for mistakes.
42 changes: 42 additions & 0 deletions mkl_fft/tests/test_patch.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
# Copyright (c) 2017, Intel Corporation
#
# Redistribution and use in source and binary forms, with or without
# modification, are permitted provided that the following conditions are met:
#
# * Redistributions of source code must retain the above copyright notice,
# this list of conditions and the following disclaimer.
# * Redistributions in binary form must reproduce the above copyright
# notice, this list of conditions and the following disclaimer in the
# documentation and/or other materials provided with the distribution.
# * Neither the name of Intel Corporation nor the names of its contributors
# may be used to endorse or promote products derived from this software
# without specific prior written permission.
#
# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
# DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE
# FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
# SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
# CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

import numpy as np

import mkl_fft
import mkl_fft.interfaces.numpy_fft as _nfft


def test_patch():
old_module = np.fft.fft.__module__
assert not mkl_fft.is_patched()

mkl_fft.patch_numpy_fft() # Enable mkl_fft in Numpy
assert mkl_fft.is_patched()
assert np.fft.fft.__module__ == _nfft.fft.__module__

mkl_fft.restore_numpy_fft() # Disable mkl_fft in Numpy
assert not mkl_fft.is_patched()
assert np.fft.fft.__module__ == old_module
Loading