Skip to content

BUG: Fix rfft for even input length.#26354

Merged
charris merged 3 commits intonumpy:mainfrom
WarrenWeckesser:fix-rfft
Apr 27, 2024
Merged

BUG: Fix rfft for even input length.#26354
charris merged 3 commits intonumpy:mainfrom
WarrenWeckesser:fix-rfft

Conversation

@WarrenWeckesser
Copy link
Member

Closes gh-26349

@WarrenWeckesser WarrenWeckesser added component: numpy.fft C++ Related to introduction and use of C++ in the NumPy code base labels Apr 26, 2024
@ngoldbaum
Copy link
Member

mac failure is an unrelated heisenbug

@ngoldbaum ngoldbaum requested a review from mhvk April 26, 2024 22:59
@charris charris added the 09 - Backport-Candidate PRs tagged should be backported label Apr 27, 2024
@charris charris added this to the 2.0.0 release milestone Apr 27, 2024
def test_rfft_even(self):
x = np.arange(8)
y = np.fft.rfft(x, 4)
assert_allclose(y, [6.0, -2.0+2.0j, -2.0], rtol=1e-14)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could use the same sort of test as for the odd case, I like comparing to the full complex fft case.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@charris charris merged commit 4340733 into numpy:main Apr 27, 2024
@charris
Copy link
Member

charris commented Apr 27, 2024

Thanks Warren.

@charris charris removed the 09 - Backport-Candidate PRs tagged should be backported label Apr 27, 2024
* for npts odd only). To make unpacking easy, we place the real data
* offset by one in the buffer, so that we just have to move R0 and
* create I0=0. Note that copy_data will zero the In component for
* create I0=0. Note that copy_input will zero the In component for
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is weird - so, the statement I made in the comment is not true? I'll try to have a quick look, but in any case the fix clearly works.

@mhvk
Copy link
Contributor

mhvk commented Apr 27, 2024

#26359 does a follow-up that ensures the code actually does what the comment states. Would be nice to have both this one and #26359 be in 2.0.0....

Must admit that overall rewriting the fft routines in terms of ufuncs has been more of an experience in how hard it is to write bug-free code than I would have liked... Sorry all!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

00 - Bug C++ Related to introduction and use of C++ in the NumPy code base component: numpy.fft

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BUG: behavioral change of numpy.fft.rfft with numpy 2.x

4 participants