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

Added Intel mkl_fft #484

Draft
wants to merge 48 commits into
base: dev
Choose a base branch
from
Draft

Added Intel mkl_fft #484

wants to merge 48 commits into from

Conversation

rohanbabbar04
Copy link
Contributor

@rohanbabbar04 rohanbabbar04 commented Feb 7, 2023

For #386

  • Added Code for FFT, FFT2D, FFTND
  • Added examples
  • Added tests

@mrava87
Copy link
Collaborator

mrava87 commented Feb 8, 2023

Thank you for this PR.

So far Mac tests are failing, do you understand why?

@rohanbabbar04
Copy link
Contributor Author

rohanbabbar04 commented Feb 8, 2023

Reason
According to this, the,mkl is not able to locate the Volumes/localdisk/tools/tc/agent1/work/636dcb48f7ee4116/base/conda-bld/numpy_and_dev_1651366109538/_build_env/lib/libmkl_rt.2.dylib' (no such file)

@mrava87
Copy link
Collaborator

mrava87 commented Feb 8, 2023

Alright can you try to investigate a bit deeper? Either it failed to install or maybe there is no bindings for mac (probably if this is the case, the mkl-fft library should say this)? Worst case scenario we can check whether it’s Mac or Linux and not run the test for Mac - we did this before, I can point you to the code if needed..

More importantly also readthedocs fails, this one we need to be able to fix before we proceed.

I’ll get back with a more detailed review soon, and @cako will probably do the same as he created the issue :)

@rohanbabbar04
Copy link
Contributor Author

Alright can you try to investigate a bit deeper? Either it failed to install or maybe there is no bindings for mac (probably if this is the case, the mkl-fft library should say this)? Worst case scenario we can check whether it’s Mac or Linux and not run the test for Mac - we did this before, I can point you to the code if needed..

More importantly also readthedocs fails, this one we need to be able to fix before we proceed.

I’ll get back with a more detailed review soon, and @cako will probably do the same as he created the issue :)

Yeah I am trying it, will let you know

@mrava87
Copy link
Collaborator

mrava87 commented Feb 8, 2023

Sounds good 👍

@mrava87
Copy link
Collaborator

mrava87 commented Feb 18, 2023

@rohanbabbar04 what is the status of this PR? I see that rtd still fails, did you find out why?

@mrava87
Copy link
Collaborator

mrava87 commented Feb 18, 2023

I see also now an issue with Python 3.10 in our Github Action. Seems like mkl-fft is not shipped for python 3.10 yet, is that possible?

@rohanbabbar04
Copy link
Contributor Author

I was able to work it out for Mac
mkl-fft is not supported for 3.10
The docs are working locally but getting seg faults here

@rohanbabbar04
Copy link
Contributor Author

I'll make a new PR explaining my changes, still need to think of the docs

@mrava87
Copy link
Collaborator

mrava87 commented Feb 18, 2023

Alright, as far as python 3.10 is concerned we could make two separate build processes with two separate requirements-dev for now...

For the doc, we definitely need to understand why before we can proceed.

Sure go ahead and make a clean PR, we can continue there :)

@rohanbabbar04
Copy link
Contributor Author

rohanbabbar04 commented Feb 18, 2023

Currently Testing, will make a new PR soon...

@cako
Copy link
Collaborator

cako commented Feb 19, 2023

I'm a bit out of the loop on this PR, what is the current status? Should I review it, or another one will be submitted? If this one is not done, I would suggest changing it to draft. Thanks!

@rohanbabbar04
Copy link
Contributor Author

I'll be making a new PR soon..

@rohanbabbar04 rohanbabbar04 marked this pull request as draft March 1, 2023 13:48
@mrava87
Copy link
Collaborator

mrava87 commented Mar 10, 2023

@rohanbabbar04 what is the status of this PR. I plan to make a new release in one week or so, shall we aim to get this one in or not?

@rohanbabbar04
Copy link
Contributor Author

Let's not include this one in the next release, currently working on my GSOC proposal
I'll finish this one after that...

@mrava87
Copy link
Collaborator

mrava87 commented Mar 10, 2023

Sounds good, thanks for letting me know :)

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.

None yet

3 participants