listmode reconstruction speed-up: Distribute sensitivity calculation and caching by NikEfth · Pull Request #1030 · UCL/STIR

NikEfth · 2022-04-27T05:24:56Z

This is the first speed-up step

* parallel LM recon * todo: corrections

NikEfth · 2022-04-27T17:34:48Z

Now, LM reconstruction is shorter by several hours.
I still need to bring here the code for the corrections.

*. The flag long_axial_fov will set the additive sinogram to either FromMemory or FromStream

NikEfth · 2022-04-29T14:52:35Z

Seriously, I don't know what its problem is. Both, in my laptop and PC it works fine.

Accidentally I brought over some TOF code

NikEfth · 2022-04-29T19:59:41Z

From this point, we can store the cache on the disk, with a very minimal footprint and restore it on the next reconstruction.
It is not very flexible, but the purposes of the TBP school should be almost alright.

KrisThielemans

quick review, not complete

KrisThielemans · 2022-04-29T22:33:48Z

oops. probably some of these review-comments are outdated (as i hadn't pressed the button 1-2 days ago)

NikEfth · 2022-04-29T22:38:19Z

No worries. But I would like to stress once more how much care you should have when you merge the TOF branch. If I fall into such trouble with a few mistaken lines .. chaos will follow. (let the record show that I write this on a Friday evening).

…nAndListModeDataWithProjMatrixByBin.cxx Co-authored-by: Kris Thielemans <[email protected]>

NikEfth · 2022-04-30T04:57:57Z

OK, I finished with your first round of comments.

robbietuk

Hopefully some of these comments can improve the code. I havent check the methodology thoroughly

Co-authored-by: robbietuk <[email protected]>

*. Fixed openmp LM guard in InputStream *. New function for caching in objective function *. Clean ups

NikEfth · 2022-05-02T04:48:21Z

Hi Robbie, thank you for your comments and suggestions. I don't know if you want me to resolve the threads (as Kris) so I leave it you, if you are happy with the response.

robbietuk

I have no major suggestions but I have not run it yet.

KrisThielemans

some house-keeping, but this almost done I believe.

2 questions:

should we rename long_axial_fov variable to reduce_memory_usage or something? It's not so obvious what it means
cache_size seems to overlap with other ways to stop. we have the num_events_to_use. Do we really need another variable?

Co-authored-by: Kris Thielemans <[email protected]>

…nAndListModeDataWithProjMatrixByBin.cxx Co-authored-by: Kris Thielemans <[email protected]>

NikEfth · 2022-05-12T12:20:48Z

cache_size seems to overlap with other ways to stop. we have the num_events_to_use. Do we really need another variable?

I made this distinction, because at some point in the future multiple cache files will be supported.

…nAndListModeDataWithProjMatrixByBin.cxx Co-authored-by: Kris Thielemans <[email protected]>

* Housekeeping comments. * Helper structure ```BinWithCorr```. With this we can read the Additive projdata in parallel, as each bin is "bundled" with its respective correction. * Parallel reading of the additive factors. Too slow otherwise. * Can load multiple additive segments at each loop. Similar to LmToProjdata's segments in memory. - But I hardcode it to 1 for now. * Can skip balanced subsets, if you know that they are balanced. * Sensible information printing on sensitivity calculation.

NikEfth · 2022-05-12T13:41:30Z

At this point, I finished with all nonTOF features. Unless, github finds something wrong.
I have 2 minor leftover todos, but only the path adjustment needs to be addressed for the Summer school.

@KrisThielemans I would like your input especially for the BinWithCorr struct in Bin.h. It allows as to read the additive factors in parallel which geatly speeds up everything. Especially, when we'll need TOF support it is essential.

KrisThielemans · 2022-05-12T21:07:35Z

Looking good. I like the BinWithCorr idea.

NikEfth · 2022-05-13T05:25:19Z

So, we are done here (until I find any bugs :P)

NikEfth

I think we are good

NikEfth added 2 commits April 27, 2022 01:16

Distribute sensitivity calculation

7c82ee9

This is the first speed-up step

Speed up LM recon

399a5dc

* parallel LM recon * todo: corrections

NikEfth added 9 commits April 28, 2022 18:21

Support for background corrections

1cb7d22

Fix include header

d85fb37

Include vector header

9d131a0

Restored the traditional LM recon when cache is set to 0

a5727e8

*. The flag long_axial_fov will set the additive sinogram to either FromMemory or FromStream

Expose long_axial_fov to the user

bdc03fc

Event counter to unsigned long int

75e6ce2

Check if is_null_ptr before trying to dynamic cast the

27d8826

Move messages somewhere less annoying.

ced14da

Remove openmp for balanced subsets.

1db33e4

NikEfth added 5 commits April 29, 2022 10:56

Dynamic_pointer_cast instead of dynamic_cast

d7a8e2d

Guard from parallel access when openMP is active and no cache used

5e22b01

Fixed a mix-up between TOF and nonTOF code

2d59345

Accidentally I brought over some TOF code

Correctly account the number of events to use

841787b

Added the ability to store/restore the cache on the disk.

062f947

Fix whether we should read the cache or skip altogether

7e93d3f

KrisThielemans linked an issue Apr 29, 2022 that may be closed by this pull request

multi-thread list mode sensitivity calculation #997

Closed

And one for appveyor

db4edda

KrisThielemans requested changes Apr 29, 2022

View reviewed changes

NikEfth and others added 2 commits April 30, 2022 00:28

Update src/recon_buildblock/PoissonLogLikelihoodWithLinearModelForMea…

ecddb15

…nAndListModeDataWithProjMatrixByBin.cxx Co-authored-by: Kris Thielemans <[email protected]>

Guard LM input from OPENMP

7d6add2

robbietuk reviewed Apr 30, 2022

View reviewed changes

Update src/include/stir/recon_buildblock/distributable.h

ce73216

Co-authored-by: robbietuk <[email protected]>

NikEfth and others added 3 commits May 1, 2022 21:06

Update src/recon_buildblock/distributable.cxx

5aa7167

Co-authored-by: robbietuk <[email protected]>

Fixes and recon_test_pack modifications for testing.

c06f3b3

*. Fixed openmp LM guard in InputStream *. New function for caching in objective function *. Clean ups

Minor fix and clean up

0cd2390

robbietuk reviewed May 3, 2022

View reviewed changes

Comment thread ..._buildblock/PoissonLogLikelihoodWithLinearModelForMeanAndListModeDataWithProjMatrixByBin.cxx Outdated

KrisThielemans requested changes May 11, 2022

View reviewed changes

NikEfth and others added 4 commits May 12, 2022 08:17

Update src/recon_buildblock/distributable.cxx

ed5aaf1

Co-authored-by: Kris Thielemans <[email protected]>

Update src/recon_buildblock/distributable.cxx

5c6ce11

Co-authored-by: Kris Thielemans <[email protected]>

Update src/recon_buildblock/PoissonLogLikelihoodWithLinearModelForMea…

b5ab9fc

…nAndListModeDataWithProjMatrixByBin.cxx Co-authored-by: Kris Thielemans <[email protected]>

Update src/recon_buildblock/PoissonLogLikelihoodWithLinearModelForMea…

1c3a8e8

…nAndListModeDataWithProjMatrixByBin.cxx Co-authored-by: Kris Thielemans <[email protected]>

NikEfth and others added 2 commits May 12, 2022 08:23

Update src/recon_buildblock/PoissonLogLikelihoodWithLinearModelForMea…

60b4f7c

…nAndListModeDataWithProjMatrixByBin.cxx Co-authored-by: Kris Thielemans <[email protected]>

No need to cache the BinWithCorr, just the Bin is fine.

c4b902f

NikEfth commented May 13, 2022

View reviewed changes

set cache_path

f2d6589

KrisThielemans added this to the v5.1 milestone May 14, 2022

KrisThielemans added the enhancement label May 14, 2022

KrisThielemans changed the title ~~Distribute sensitivity calculation~~ listmode reconstruction speed-up: Distribute sensitivity calculation and caching May 14, 2022

KrisThielemans merged commit 8a83e48 into UCL:master May 14, 2022

KrisThielemans added a commit that referenced this pull request May 15, 2022

fix bug in ROOT loop introduced in #1030

a4c8283

Conversation

NikEfth commented Apr 27, 2022

Uh oh!

NikEfth commented Apr 27, 2022

Uh oh!

NikEfth commented Apr 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

NikEfth commented Apr 29, 2022

Uh oh!

KrisThielemans left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

KrisThielemans commented Apr 29, 2022

Uh oh!

NikEfth commented Apr 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

NikEfth commented Apr 30, 2022

Uh oh!

robbietuk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

NikEfth commented May 2, 2022

Uh oh!

robbietuk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

KrisThielemans left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

NikEfth commented May 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

NikEfth commented May 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

KrisThielemans commented May 12, 2022

Uh oh!

NikEfth commented May 13, 2022

Uh oh!

NikEfth left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

NikEfth commented Apr 29, 2022 •

edited

Loading

NikEfth commented Apr 29, 2022 •

edited

Loading

NikEfth commented May 12, 2022 •

edited

Loading

NikEfth commented May 12, 2022 •

edited

Loading