WIP for continuous mode

[adriaangraas]

WIP pull request for #4 for the purpose of feedback and testing.

[adriaangraas]

There was actually no need for a double buffer_ variable. That, and for alternating mode I set the update_every to the group_size. This simplified code quite a bit as write_index_ was not necessary to index buffer_ anymore.

[jwbuurlage]

Thanks again for implementing this! Let me know when this is ready for a proper review.

Please see the TODO list above, and please also make sure that the old functionality still works the same :)

[adriaangraas]

In about two weeks I will properly test this, add some comments here and there and do the formal wrap up.

[jwbuurlage]

I have updated the processing system. After implementing Paganin filtering support, the processing function was becoming a mess.

Instead, there is now a ProjectionProcessor which has a number of (optional) components. The parallelization is now over full projections, which should also make it easier for you to have an alternating/continuous mode by simply calling projection_processor_.process with the proper data position and projection count. The system is much better/flexible/modular this way.

I figured this was better to do pre-merge than post-merge. I don't think it should take too much time for you to adapt your changes to the new system.

[adriaangraas]

There are only three buffers now, the buffer for incoming data (processing is in-place there), the sino_buffer which is the transpose of the buffer, and the GPU buffer. Should be easy enough to document once we get there.

I also made a few changes to the code that made the difference between alternating and continuous mode more apparent, so that it is easier to understand and debug in the future.

I made a CHANGELOG entry, but I suggest that you make an AUTHORS file?

I've tested the different modes here and they seem to work alright. Would you like to give the code a try at your system?

[jwbuurlage]

At first glance this looks very reasonable :+1: I will do a detailed review next week, and test it on my own system. Thanks!

[adriaangraas]

I think this is also the opportunity to change the behavior of --group-size in the alternating mode a bit.

Currently, if you specify group-size, this will override the size of proj_count from the geometry specification packet. If you send in 600 projections per rotation, and select a 300 group-size, then you will find your reconstruction rotated by 180 degrees every other rotation.

I propose to make --group-size purely the size of the group that is simultaneously processed. Then we have:

• --group-size the amount of simultaneous processed projections
• --update-every the size of the CPU buffer, and how often projections are uploaded to the GPU buffer
• geom.proj_count the size of the GPU buffer (of which we have two in alternating mode)

Then --countinuous writes in one buffer, and locks and alternating just has two buffers and switches writing to either one. The flag then does not alter any other functionality.

Currently in alternating mode, --group-size equals --update-every, but I think it'd be nice to be able to set the processing/upload batch size separately in this mode as well.

[jwbuurlage]

I finally got around to reading through the code, nothing sticks out to me except for the minor comments I gave before.

I think there are still some things unclear about group_size and update_every.

In my original design:

• the group size was about parallel computation: how coarsely do we want to perform our parallelism. I wanted at least a certain amount of data, so that each thread would have enough work to do.
• the parameter update every was about refreshing reconstructions, regardless of computation. How often do we want to refresh the (outdated) slices and 3D preview.

In alternating mode, we want the reconstruction to build up instead of waiting for reconstruction until we have received the first full rotation (this is particularly relevant to slow lab setups like ours). After this, we want to refresh only after every full rotation. The way this worked is by updating the reconstruction every 'group_size (= update_every)' projections during the first rotation, and only every 'proj_count' in any subsequent rotation. I think this functionality would still make sense.

In continuous mode, I think the logical way is still to have 'group_size = update_every', but for all rotations, not only the first one. So maybe, the extra flag 'update every' is actually not necessary.

For the parallel beam, I think your issue is that you should only send projections in the angular range [0, \pi]. See also my comment on the tomopy script.

[jwbuurlage]

Is this ready for a final check + merge?

[adriaangraas]

Do we want to remove the update_every from the argument list, and just automatically set it to the sensible defaults (i.e. equal to group_size)?

[jwbuurlage]

In my opinion, yes

[adriaangraas]

Okay, issue #9 was implicitly fixed in this branch and I've removed the update_every param. Things seem to run fine here, both with/without --continuous and --group-size. Would you like to make the final check before the merge?

[jwbuurlage]

Cool! Thanks.