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

Detect if length of array is too much for MKL to handle, warn if it is #4405

Conversation

GarethCabournDavies
Copy link
Contributor

@GarethCabournDavies GarethCabournDavies commented Jun 14, 2023

In #4394 we moved to MKL as first preference, but #4395 shows that this is not always best - here we catch that issue and switch to fftw if it is appropriate add a warning

@josh-willis
Copy link
Contributor

Please, please, please don't do this in this manner.

This is hard-coding in the lower levels of our infrastructure a current limitation of one FFT backend which could then change. And it's making a decision for the user who then has no way to override it.

We should raise the exception, and allow executables to choose to catch the exception if they can.

@GarethCabournDavies
Copy link
Contributor Author

Would this be okay if I add in a test for whether the mkl backend was chosen as a default or as a specific choice through the cli? Then:

  1. If the backend was set, don't change anything (but add a logging.warning)
  2. If the default was used, then we will warn and switch

@josh-willis
Copy link
Contributor

I just don't think this should be handled at the library level. Are you (personally) going to watch MKL release notes to know if this should change? Hard-coding this kind of thing is a mistake from a software-engineering point of view.

And I don't think there's a ready way to put this check where you want and to detect whether the backend was specifically requested or defaulted. There really shouldn't be a difference. If an end-user of the library is agnostic about the FFT backend, then they should put a try/except block in their executable.

@GarethCabournDavies
Copy link
Contributor Author

Okay, I will add in a warning, but nothing more

@GarethCabournDavies GarethCabournDavies changed the title Detect if length of array is too much for MKL to handle, switch to FFTW if it is Detect if length of array is too much for MKL to handle, warn if it is Jun 15, 2023
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.

2 participants