Skip to content

MR: Speed up FourierEncoding forward and backward - #1409

Draft
paskino wants to merge 2 commits into
SyneRBI:masterfrom
paskino:omp-mr-acqmodel
Draft

MR: Speed up FourierEncoding forward and backward#1409
paskino wants to merge 2 commits into
SyneRBI:masterfrom
paskino:omp-mr-acqmodel

Conversation

@paskino

@paskino paskino commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Changes in this pull request

  • Reordered for loops in forward and backward of FourierEncoding.cpp to match shape from ISMRMRD.
  • Added OMP directive

https://github.com/ismrmrd/ismrmrd/blob/65fa1d6a7522535c2302ad854224095c0d213aae/libsrc/ismrmrd.cpp#L1140-L1146

Testing performed

On the VM there is a sensible speed up in d_undersampled_reconstruction.ipynb from SIRF-Exercises

Num threads Time per forward+backward [s]
1 master 9
1 this PR 6.3
2 7.4

Remains to test on a larger dataset. Probably the fact that the OMP threads are not useful comes from the fact that the test I did is with a very tiny dataset and the cost of thread creation is larger than the compute gain.

Related issues

Checklist before requesting a review

  • I have performed a self-review of my code
  • I have added docstrings/doxygen in line with the guidance in the developer guide
  • I have implemented unit tests that cover any new or modified functionality
  • The code builds and runs on my machine
  • CHANGES.md has been updated with any functionality change

Contribution Notes

Please read and adhere to the contribution guidelines.

Please tick the following:

  • The content of this Pull Request (the Contribution) is intentionally submitted for inclusion in SIRF (the Work) under the terms and conditions of the Apache-2.0 License.

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

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...

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants