TomographicImaging / CIL

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

Change in behaviour of PDHG #1796

Open paskino opened 4 months ago

paskino commented 4 months ago

Description

From @samtygier-stfc on Discord

I am upgrading from 23 -> 24, and came across some changes in the way we iterate the PDHG algorithm. Previously we created the PDHG instance with a large number for max_iterations, and then had our own loop to which called next() to run each iteration

algo = PDGH(..., max_iterations=1000000, ...)
for i in range(N):
    algo.next()
    ...

In 24.0 the next() method is gone, but I can do next(algo) instead. Also max_iterations is deprecated in the constructor, with the suggestion to use Algorithm.run(iterations). If I remove max_iteration from the constructor, then next() will give StopIteration straight away. I can get around the deprecation warning by doing

algo = PDGH(...)
algo.max_iterations = 1000000
for i in range(N):
    next(algo)
    ...
paskino commented 4 months ago

So far algo.max_iteration is used, although we would like to discourage it in favour of the algo.run method with callbacks. Also, next has been removed (it was there for compatibility with Python2) but __next__ is still there.

casperdcl commented 4 months ago

For now you could use:

algo = PDGH(...)
algo.max_iterations = 1000000
for i in iter(algo):
    ...  # do something with algo

but it might stop working in a future release.

Best is:

from cil.optimisation.utilities.callbacks import Callback, ProgressCallback

class EachIterCallback(Callback):
    def __call__(self, algo):
        ...  # do something with algo

algo = PDHG(...)
algo.run(1000000, callbacks=[ProgressCallback(), EachIterCallback()])

vis. https://tomographicimaging.github.io/CIL/v24.0.0/optimisation/#callbacks

If you've made any custom callbacks you think others will benefit from (e.g. TensorboardCallback or something) please do open a PR adding it to Wrappers/Python/cil/optimisation/utilities/callbacks.py!

samtygier-stfc commented 4 months ago

I've switched to doing the progress bar updates from a callback: https://github.com/mantidproject/mantidimaging/pull/2184/commits/5006fd30d5ae81a89c7904d32af9af71cb175585

It would be good if there was a mention in the release notes about the change.

casperdcl commented 4 months ago

I've switched to doing the progress bar updates from a callback: mantidproject/mantidimaging@5006fd3

lgtm!

It would be good if there was a mention in the release notes about the change.

Unfortunately it is mentioned in the highly inaccessible CIL@master:/CHANGELOG.md but for some unknown reason @TomographicImaging/cil-developers seem to dislike keeping the more standard CIL/releases readable/helpful/neat/up-to-date/in-sync ðŸ˜