-
Notifications
You must be signed in to change notification settings - Fork 22
add manual NumPy patching to mkl_fft
#224
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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
|
||||||||||||||||||
| _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
AI
Feb 20, 2026
There was a problem hiding this comment.
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.
| 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
AI
Feb 20, 2026
There was a problem hiding this comment.
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.
| self._restore_func(name, verbose=verbose) | |
| self._restore_func(name, verbose=verbose) | |
| self._restore_dict.clear() |
Copilot
AI
Feb 20, 2026
There was a problem hiding this comment.
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.
| 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
AI
Feb 20, 2026
There was a problem hiding this comment.
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.
| return (getattr(_tls, "initialized", None) is not None) and ( | |
| _tls.initialized is True | |
| ) | |
| return getattr(_tls, "initialized", False) |
Copilot
AI
Feb 20, 2026
There was a problem hiding this comment.
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
AI
Feb 20, 2026
There was a problem hiding this comment.
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
AI
Feb 20, 2026
There was a problem hiding this comment.
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.
| 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 |
Uh oh!
There was an error while loading. Please reload this page.