Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Move from 2d to 3d array operations #12
base: main
Are you sure you want to change the base?
Move from 2d to 3d array operations #12
Changes from 40 commits
e9a623a
284ebe1
d1c197d
feed1b5
c6a03c9
32fcbf9
3f2cf26
a484b98
3ec047b
16a2b43
8e434ea
3f6c0c4
7e26aef
ffbb2c8
fb205f8
369efe9
24c5588
5f92921
32e71e1
dc96bc8
fc91620
d3ba3fb
286ed5a
da0f046
10c59b6
02dd9c0
44d62ba
5a156d7
222bf80
1b799f8
eb21e3f
0cba1d2
776783c
6d0a6cf
4707ffc
fe5e62a
5aa0885
560054c
880591a
b64aa82
d46e0d7
ae89268
259f946
e0ec9d2
5c390da
5553c2f
af8fc85
84a5732
528dc3a
e1cf228
bd671d7
1d3a22a
b91f549
ba6c652
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 problem here is that you are not taking into account FFTW wisdom (https://en.wikipedia.org/wiki/FFTW). FFTW should be much faster than numpy, because it initially tests several FFTs on the input data shape and then takes the fastest one. The wisdom is forgotten unless you store it locally on disk everytime you use pyfftw (#5).
I assume that if you added PyFFTW a second time to the
fft_interfaces
list, you will get faster results (because the wisdom is already there from the first run).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.
I can add a third one without the initial wisdom
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.
FYI, the wisdom only is a factor in the initial batch 8 run:
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.
Something is not right here. The FFTW with wisdom must always be faster than the one without wisdom. Did you measure the time with
time.perf_counter()
?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.
It could also be that we are not using PyFFTW correctly in qpretrieve. I always thought wisdom is handled automatically.
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.
Isn't what we are seeing here that the wisdom is calculated during the first loop (batchsize 8), and thereafter Pyfftw uses this for all further calculations?
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.
PyFFTW should compute the wisdom for every batch size individually. For batch sizes 8 and 256 the one with wisdom is slower than the one without wisdom. I would not have expected that.
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.
I think you did not use
time.perf_counter
in your script. Maybe you can try that. It could explain this, since you are normalizing by batch size.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.
Here I have compared pyfftw (not including wisdom calulation time) with numpy. Should be resolved now.