-
Notifications
You must be signed in to change notification settings - Fork 13
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
fft notebook #409
fft notebook #409
Conversation
@lang-m When you get a chance could you please look through this. There may be a bug in So the current example is probably more useful for people doing micromagnetics but I also considered changing the example to 2 spatial dimensions and a scalar field as this simplifies the code/plots a little. |
I have fixed the bug in #410. Please review that first and then merge into this one then please review this. |
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 notebook looks really good. Here is some preliminary feedback from a first review. I'll go over it again more thoroughly after reviewing and merging the bug fix.
Codecov ReportPatch coverage:
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. Additional details and impacted files@@ Coverage Diff @@
## master #409 +/- ##
==========================================
- Coverage 96.87% 93.35% -3.53%
==========================================
Files 28 28
Lines 2498 2933 +435
==========================================
+ Hits 2420 2738 +318
- Misses 78 195 +117
☔ View full report in Codecov by Sentry. |
no fftshift on last dim for real ffts
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.
Final review for the first part of the notebook (anything before "Visualising and analysing FFTs"). I have added some more suggestions and minor corretions.
I'll try to also review the second half of the notebooks soon.
Co-authored-by: Martin Lang <67915889+lang-m@users.noreply.github.com>
WalkthroughThe changes made to the codebase focus on improving the Fast Fourier Transform (FFT) functionality in the Changes
Uplevel your code reviews with CodeRabbit ProCodeRabbit ProIf you like this project, please support us by purchasing the Pro version. The Pro version has advanced context, superior noise reduction and several proprietary improvements compared to the open source version. |
Co-authored-by: Martin Lang <67915889+lang-m@users.noreply.github.com>
Co-authored-by: Martin Lang <67915889+lang-m@users.noreply.github.com>
Co-authored-by: Martin Lang <67915889+lang-m@users.noreply.github.com>
Co-authored-by: Martin Lang <67915889+lang-m@users.noreply.github.com>
Co-authored-by: Martin Lang <67915889+lang-m@users.noreply.github.com>
Co-authored-by: Martin Lang <67915889+lang-m@users.noreply.github.com>
Co-authored-by: Martin Lang <67915889+lang-m@users.noreply.github.com>
Co-authored-by: Martin Lang <67915889+lang-m@users.noreply.github.com>
Co-authored-by: Martin Lang <67915889+lang-m@users.noreply.github.com>
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.
Some additional minor comments for the second half of the notebook.
Feel free to merge once all open comments are addressed.
Edit: several of the comments seem to be off by one line (not sure how that has happened). I hope they all make sense. Directly applying the suggestions of course doesn't work for these.
Co-authored-by: Martin Lang <67915889+lang-m@users.noreply.github.com>
Dependent on:
Summary by CodeRabbit
scipy
library and optimizing computation.discretisedfield/mesh.py
.discretisedfield/tests/test_field.py
, ensuring correctness in various scenarios.axes
parameter in certain functions ofdiscretisedfield/field.py
to exclude the last axis, improving computation efficiency.scipy.fft
module asspfft
indiscretisedfield/tests/test_field.py
.These changes collectively improve the performance and reliability of the code while maintaining compatibility with the external interface.