Skip to content
Snippets Groups Projects

Memory fix C code

Merged Yang, Diyu requested to merge memory_fix_python into master

Created by: dyang37

This PR completes the memory fix in C code, plus some memory-related improvements in Cython interface. It contains the following fixes:

  1. Installed new multialloc routine, and replaced all mem_alloc_2D and mem_alloc_3D functions with multialloc. This is to handle issue #10 (closed) .
  2. In C code, remove unnecessary copy of pointer arrays for img.vox, img.proxMapInput, sino.vox, sino.wgt in interface.c.
  3. In Cython interface, initialize recon image directly with init image. This avoids unnecessary memory re-allocation of the recon image.
  4. In C code, added functionality of checking whether prox image and recon image are from the same memory address, and make a copy when necessary. This is to handle the corner case that might occur after fix 3, where x_init and prox_image might be the same variable.
  5. In Cython interface, only make contiguous array when necessary.

Testing

  1. recon testing: Cleared sys matrix cache, and ran demo_3D_shepp_logan.py and demo_mace3D.py. Results for both experiments are visually the same as before.
  2. Unit test for fix 4: When doing proximal map mode recon, provide the same variable input of init image and proximal image. The memory address detection mechanism is triggered (as expected).

Merge request reports

Approval is optional

Merged by avatar (Mar 13, 2025 5:06pm UTC)

Merge details

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Created by: dyang37

    Tested the memory usage with demo_3D_shepp_logan.py. For a large reconstruction problem where sino shape = 144x512x512, recon shape = 799x473x473, the overall memory usage drops from 8615MB (current master branch code) to 7929MB (PR code). Edit Memory usage drops to 7248MB after fixing the unnecessary proximal image memory issue in commit 5a72a033.

  • Created by: cabouman

    Jordan, Diyu made the memory fixes. Can you look this over and see if it is OK?

    We are the only people using this code right now, so we don't have to be too cautious, but I just want to check that the changes look reasonable and that it works.

  • Created by: sjkisner

    There's changes to allocate.c and allocate.h which come from https://github.com/cabouman/C-code If there's necessary changes here you probably want to make them to the master versions first.

  • Created by: dyang37

    There's changes to allocate.c and allocate.h which come from https://github.com/cabouman/C-code If there's necessary changes here you probably want to make them to the master versions first.

    allocate.c and allocate.h are changed to be exactly the same as in https://github.com/cabouman/C-code. Previously we were using some non-standard memory alloc code written by Soumendu and Thilo. Should I make a PR that contains only changes in allocate.c and allocate.h first, and then make a separate PR for the rest of the changes?

  • Created by: sjkisner

    Oh, nevermind. I didn't realize you changed allocate.{c,h} to the version in cabouman/C-code

  • Created by: sjkisner

    I'd suggest not calling a function when you index like this: sino->e[idx_3D_to_1D(i_beta,i_v,i_w,sino->params.N_dv,sino->params.N_dw)] and Ax[idx_3D_to_1D(i_beta,i_v,i_w,sinoParams->N_dv,sinoParams->N_dw)]

    The overhead in calling a function everywhere you index will add up. I'd suggest either putting the expression directly for the index, or defining idx_3D_to_1D() as a macro rather than a function.

  • Created by: dyang37

    I'd suggest not calling a function when you index like this: sino->e[idx_3D_to_1D(i_beta,i_v,i_w,sino->params.N_dv,sino->params.N_dw)] and Ax[idx_3D_to_1D(i_beta,i_v,i_w,sinoParams->N_dv,sinoParams->N_dw)]

    The overhead in calling a function everywhere you index will add up. I'd suggest either putting the expression directly for the index, or defining idx_3D_to_1D() as a macro rather than a function.

    Thank you Jordan! That makes sense. I changed to indexing function to be a Macro. Please see the latest commit e29ec098.

  • Merged by: cabouman at 2022-04-28 11:41:25 UTC

Please register or sign in to reply
Loading