Ticket #7586 (closed: fixed)
Inverse FFT on matrix
Reported by: | Arturs Bekasovs | Owned by: | Dan Nixon |
---|---|---|---|
Priority: | major | Milestone: | Release 3.3 |
Component: | GUI | Keywords: | |
Cc: | Blocked By: | ||
Blocking: | Tester: | Harry Jeffery |
Description (last modified by Arturs Bekasovs) (diff)
While in MantidPlot, I create a new matrix (NOT MatrixWorkspace), resize it to be of size 3x3 and fill with some values, e.g. 1-9. Then I open the Analysis menu, and select Inverse FFT. It crashes the MantidPlot. Happens if you run the FFT... with Inverse option as well.
Observed in 2.5.1960 on Ubuntu 12.04 64bit.
Change History
comment:2 Changed 7 years ago by Andrei Savici
- Status changed from new to verify
- Owner set to Andrei Savici
- Resolution set to worksforme
- Milestone changed from Backlog to Release 3.1
Works for me in Ubuntu 13.10 and RHEL 6 (Mantid 3.0). I don't see InverseFFT, but works for FFT and select Inverse
comment:3 Changed 7 years ago by Michael Reuter
- Status changed from verify to verifying
- Tester set to Michael Reuter
comment:4 Changed 7 years ago by Michael Reuter
- Status changed from verifying to reopened
- Resolution worksforme deleted
I tested using the latest master on all platforms and here's what I found:
Inverse FFT | FFT with Inverse Opt | |
---|---|---|
RHEL6 | Works | Works |
Windows 7 | Crashes | Crashes |
OS X 10.8 | Crashes | Crashes |
Ubuntu 12.04 | Crashes | Works |
Looks like this does not work reliably.
comment:5 Changed 7 years ago by Andrei Savici
- Owner changed from Andrei Savici to Anyone
- Milestone changed from Release 3.1 to Backlog
comment:6 Changed 6 years ago by Dan Nixon
- Status changed from reopened to inprogress
- Owner changed from Anyone to Dan Nixon
comment:7 Changed 6 years ago by Dan Nixon
The Analysis>FFT... dialogue seems to use GSL FFT whereas the Analysis>Forward FFT and Analysis>Inverse FFT use fft2D.cpp.
I have corrected an error in fft2D.cpp which stops a crash on Ubuntu 14.04 and compared results using both FFT algorithms and found them to match.
Will move to testing it on Windows.
comment:9 Changed 6 years ago by Dan Nixon
Analysis>Inverse FFT now works on Ubuntu and I'm confident is should on other platforms. I could do with this being tested on both Windows 7 and OS X to confirm.
I haven't been able to test the issue with Analysis>FFT... causing crashes.
comment:10 Changed 6 years ago by Dan Nixon
- Status changed from inprogress to verify
- Resolution set to fixed
comment:11 Changed 6 years ago by Dan Nixon
Note this needs to be tested on Ubuntu, Windows and OS X before being closed.
comment:12 Changed 6 years ago by Peter Peterson
- Milestone changed from Backlog to Release 3.3
Should be on the current release
comment:13 Changed 6 years ago by Nick Draper
- Status changed from verify to verifying
- Tester changed from Michael Reuter to Nick Draper
comment:14 Changed 6 years ago by Nick Draper
- Status changed from verifying to verify
- Tester Nick Draper deleted
Tested and works on Windows 7, Needs someone to test on the Mac now really
comment:15 Changed 6 years ago by Michael Reuter
- Status changed from verify to verifying
- Tester set to Michael Reuter
comment:16 Changed 6 years ago by Michael Reuter
The FFT menu options seems to work with all options now on OSX and I check RHEL6 and Fedora 20 for good measure. However, the Forward FFT and Inverse FFT menu options no longer seem to do anything relevant on any platform (OSX, Windows, RHEL6, Fedora 20). Having a matrix and then invoking one of those options just clears the matrix. Not very helpful. This used to work on RHEL6. Should this be fixed on this ticket or should another one be generated?
comment:17 Changed 6 years ago by Dan Nixon
I'm not able to reproduce that issue on Ubuntu 14.04, both the forward and inverse FFT menu options produce an FFT in the original matrix (unless you refer to the fact that the original matrix is overwritten?)
comment:18 Changed 6 years ago by Michael Reuter
The original matrix is not overwritten but simply disappears and no results are generated.
comment:19 Changed 6 years ago by Dan Nixon
Since the FFT menu seems to be fine now is the easiest option here to just remove the Forward and Reverse FFT menu options since they only seem to duplicate what can be done in the FFT menu?
comment:20 Changed 6 years ago by Dan Nixon
- Status changed from verifying to reopened
- Resolution fixed deleted
comment:21 Changed 6 years ago by Dan Nixon
I tried the "Forward FFT" and "Inverse FFT" options under Windows and they both seem to work fine for me.
comment:22 Changed 6 years ago by Dan Nixon
Just realised what the problem is here, the matrix is only cleared when it contains empty cells.
A warning and doing nothing would be better in this case.
comment:23 Changed 6 years ago by Dan Nixon
- Status changed from reopened to inprogress
Merge branch 'master' into bugfix/7586_matrix_inverse_fft_fix
Refs #7586
Changeset: 8e3b7bc8a745d2cb630cd7d60d18afb1d33b54a6
comment:24 Changed 6 years ago by Dan Nixon
Warn when martix has NaN values before FFT
Refs #7586
Changeset: 03073e3902588cf6d09b99407872229dabcc00ee
comment:25 Changed 6 years ago by Dan Nixon
To test:
- Create a matrix, resize it and add some data such that every cell has a value
- Run the "Forward FFT..." and "Inverse FFT..." from the Analysis menu
- Repeat step 1 but leave some cells blank
- Run the "Forward FFT..." and "Inverse FFT..." from the Analysis menu, this should give a warning and leave the original data intact
comment:26 Changed 6 years ago by Dan Nixon
- Status changed from inprogress to verify
- Resolution set to fixed
comment:27 Changed 6 years ago by Harry Jeffery
- Status changed from verify to verifying
- Tester changed from Michael Reuter to Harry Jeffery
comment:28 Changed 6 years ago by Dan Nixon
- Status changed from verifying to closed
Merge branch 'master' into bugfix/7586_matrix_inverse_fft_fix
Refs #7586
Full changeset: 8e3b7bc8a745d2cb630cd7d60d18afb1d33b54a6
comment:29 Changed 6 years ago by Harry Jeffery
Merge remote-tracking branch 'origin/bugfix/7586_matrix_inverse_fft_fix'
Full changeset: 417a4b16c08dc53c87d2bb74ef5738997a0c9e6a
comment:30 Changed 5 years ago by Stuart Campbell
This ticket has been transferred to github issue 8431