Fix hfftn/ihfftn passing garbage dim values when dim is null#1563
Merged
alinpahontu2912 merged 5 commits intoMay 4, 2026
Conversation
alinpahontu2912
force-pushed
the
fix-hfft-real-input-test
branch
from
ef72c4f to
4bfd13b
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a native interop bug in TorchSharp’s FFT bindings where hfftn/ihfftn could receive invalid (“garbage”) dimension values when dim is omitted, aligning default-dimension behavior with the fftn/ifftn pattern and updating tests accordingly.
Changes:
- Update native
THSTensor_hfftn/THSTensor_ihfftnto avoid passing invalid defaultdimvalues whendim == null. - Replace unsafe initializer-list based default dims in several 2D FFT wrappers with a stable local array-backed
IntArrayRef. - Adjust/rename FFT unit tests to use complex inputs for Hermitian FFT APIs and to call the correct
hfftn/ihfftnentry points.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/Native/LibTorchSharp/THSFFT.cpp |
Fixes default dim handling for hfftn/ihfftn and stabilizes default 2D dim storage to prevent dangling references. |
test/TorchSharpTest/TestTorchTensor.cs |
Updates Hermitian FFT tests to use complex inputs and corrects coverage for hfftn/ihfftn default-dim behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
alinpahontu2912
force-pushed
the
fix-hfft-real-input-test
branch
from
4bfd13b to
30745f5
Compare
PyTorch's fft.hfft/hfft2/hfftn require complex input tensors. The tests were passing real-valued tensors (Float32/Float64), which causes 'NYI' errors in newer PyTorch versions. Fixed by using complex64/complex128 input types and ihfft for inverse verification. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The test method was incorrectly calling fft.hfft2() instead of fft.hfftn(), and using ihfft2() instead of ihfftn() for verification. Updated the TestOf attribute to match the correct function being tested. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Removed Float32HFFT and Float64HFFT as they were duplicates of ComplexFloat versions. Renamed Float32HFFT2/N and Float64HFFT2/N to ComplexFloat... to reflect input type. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The FFT functions (fft2, ifft2, hfft2, ihfft2, hfftn, ihfftn) created c10::IntArrayRef from a temporary initializer_list in a ternary expression. The backing data was destroyed at the end of the expression, leaving a dangling pointer. On Windows this caused garbage dimension values (e.g. 740900711656) to be passed to PyTorch, triggering 'Dimension out of range' errors. Fixed by declaring the default dim array with proper lifetime before the ternary expression. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
When dim is null, construct all-dimensions array (matching Python's
dim=None behavior) instead of relying on PyTorch's C++ default {-2,-1}.
Unlike fftn/ifftn which take OptionalIntArrayRef for dim, hfftn/ihfftn
take IntArrayRef (non-optional), so nullopt cannot be used. Building
the full dim range from the tensor's ndim correctly matches the Python
API semantics.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
alinpahontu2912
force-pushed
the
fix-hfft-real-input-test
branch
from
30745f5 to
6daf54f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Problem
ComplexFloat32HFFTN test fails with:
Dimension out of range (expected to be in range of [-4, 3], but got 464974753960)
Root Cause
THSTensor_hfftn and THSTensor_ihfftn hardcoded a 2-element default dim array {-2, -1} when dim was null. This pattern (copied from the 2D variants hfft2/ihfft2) is incorrect for the n-dimensional variants, which should pass c10::nullopt to let PyTorch use all dimensions, matching fftn/ifftn.
Fix
Replace the hardcoded default with c10::nullopt, matching the fftn/ifftn pattern.