TomographicImaging / CIL

A versatile python framework for tomographic imaging
https://tomographicimaging.github.io/CIL/
Apache License 2.0
97 stars 44 forks source link

Unnecessarily copying data each iteration #622

Closed gfardell closed 2 years ago

gfardell commented 4 years ago

In FISTA and PDHG we copy x to x_old every iteration adding unnecessary overhead.

Instead we should pre-allocate 2 containers, and store a new variable referencing each of them. Each interaction we swap the reference, not the data.

Current:

Alg setup:
    x = allocate()
    x_old = allocate()

Alg update:
    x_old.fill(x)
    x = F(x_old)

Proposed:

Alg setup:
    A = allocate()
    B = allocate()
    x_old = A
    x = B

Alg update:
    x_temp = x_old
    x_old = x
    x = x_old

    x = F(x_old)
paskino commented 4 years ago

Possibly this should be done at the algorithm level so that the user doesn't even have to bother with it? Let's add it as a special method of the algorithm class. CGLS doesn't use it, for instance, but do we mind having it?

gfardell commented 4 years ago

I'm not sure we want to add to the memory usage unnecessarily for CGLS.

It would be good for the algorithm to handle it though. We could have an Algorithm member function that allocates 2 copies of the data container (probably as a block) and returns the 2 handles we want them to use (and keeps the 2 real references private). Then the algorithm could handle swapping them every iteration if they exist. We need it to be able to handle more than one variable and variable_prev pair and not assume it's the solution.

gfardell commented 3 years ago

Need to check the state of the algorithms

gfardell commented 3 years ago

941 optimised the memory usage of PDHG, it already had the pointer switch but there were some more savings. I think this issue should be expanded to look at each algorithm implementation.