Skip to content

MR: Speed up FourierEncoding forward and backward #1409

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.

Sign up for GitHub

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

Draft
paskino wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

MR: Speed up FourierEncoding forward and backward #1409

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9354468
add omp directives to FourierEncoding
paskino Jun 22, 2026
78c4001
updates for build
paskino Jun 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ else()
find_package(ISMRMRD 1.4.2 REQUIRED)
# Add ISMRMRD to search path for FFTW3
set(CMAKE_MODULE_PATH "${CMAKE_MODULE_PATH};${ISMRMRD_DIR}")
find_package(FFTW3 COMPONENTS single REQUIRED)
find_package(FFTW3 COMPONENTS single double threads REQUIRED)
ADD_SUBDIRECTORY(xGadgetron)
set(SIRF_BUILT_WITH_ISMRMRD TRUE PARENT_SCOPE)
set(ISMRMRD_VERSION ${ISMRMRD_VERSION} PARENT_SCOPE)
Expand Down
4 changes: 4 additions & 0 deletions src/Registration/cReg/NiftyAladinSym.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,11 @@ limitations under the License.
#include "sirf/Reg/NiftiImageData3D.h"
#include "sirf/Reg/NiftiImageData3DDeformation.h"
#include "sirf/Reg/NiftiImageData3DDisplacement.h"
// Compatibility with NiftyReg builds where _reg_aladin.h still references
// InputTransform while only TransformationMatrix is declared.
#define InputTransform TransformationMatrix
#include <_reg_aladin_sym.h>
#undef InputTransform

using namespace sirf;

Expand Down
2 changes: 2 additions & 0 deletions src/xGadgetron/cGadgetron/FourierEncoding.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ void sirf::CartesianFourierEncoding::forward(MRAcquisitionData& ac, const CFImag
int kz = nz/2 - kz_lim.center + acq.idx().kspace_encode_step_2;

for (unsigned int c = 0; c < nc; c++) {
#pragma omp parallel for
for (unsigned int s = 0; s < nx; s++) {
acq.data(s, c) = ci(s, ky, kz, c);
}
Expand Down Expand Up @@ -212,6 +213,7 @@ void sirf::CartesianFourierEncoding::backward(CFImage& img, const MRAcquisitionD
int z = nz/2 - kz_lim.center + acq.idx().kspace_encode_step_2;

for (unsigned int c = 0; c < nc; c++) {
#pragma omp parallel for

@KrisThielemans KrisThielemans Jun 22, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

TBH, I'm not sure if this is thread-safe. I've read some things about aligning etc (to do +=, you need to fetch, compute, store, which if your computer does the fetch/store in a larger chunk than the data element, can get you into trouble). Can't find the reference though.

Alternative is atomic, but obviously slower.

BTW, you can collapse loops, but not in Visual Studio C++ (it support OpenMP 2), so you need some ugly ifdefs which I can point you to.

Also, in most compilers, #pragma omp will lead to compilation warnings when not using OpenMP, so in STIR, we put some more #ifdef around it...

for (unsigned int s = 0; s < readout; s++) {
ci(s, y, z, c) += acq.data(s, c);
}
Expand Down
Loading