-
Notifications
You must be signed in to change notification settings - Fork 0
Added complex manipulation ops and unit tests for added ops #2
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
Conversation
My suggestions:
|
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.
Looks great, I've suggested a few modifications, let me know what you think and I can implement them if you agree :-)
Ok I've made the executive decision of adding the code for the transpose inside the base class, the logic is this particular order of transpose is connected to the rest of the fft algorithm, so kind of awkward to have it alone in a different class |
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.
LGTM, for some reasons the tests are not running automatically on github, I'm gonna try to ru them on my machine and after that it's all good
ok, all good. Thanks @zaccharieramzi ! |
This PR adds:
fft3d
andifft3d
)The tests are passing in the current state, but I am going to suggest in a comment underneath some modifications to the current code.