-
Notifications
You must be signed in to change notification settings - Fork 44
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
SIRT update and objective methods #1514
Comments
Hi @epapoutsellis! Thanks for this. I have been taking a look this afternoon - do you have any examples of using TV with SIRT? I am trying to compare with FISTA but the SIRT algorithm seems to be producing just zeros: data = dataexample.SIMPLE_PHANTOM_2D.get(size=(128,128))
ig = data.geometry
A=IdentityOperator(ig)
constraint=TotalVariation()
initial=ig.allocate('random', seed=5)
sirt = SIRT(initial = initial, operator=A, data=data, max_iteration=100, constraint=constraint)
sirt.run(100, verbose=2)
f=LeastSquares(A,data, c=0.5)
fista=FISTA(initial=initial,f=f, g=constraint, max_iteration=100)
fista.run(100, verbose=2)
self.assertNumpyArrayAlmostEqual(fista.x.as_array(), sirt.x.as_array())
self.assertAlmostEqual(fista.loss[-1], sirt.loss[-1]) |
I think this test is failing because of #1650 |
Yes, just tested it. It was working at some point. Even with ISTA + WeightedLS + Preconditioner fails, so I think is due to #1650 |
But it does work with alpha = 1e-6
G = alpha * FGP_TV(max_iteration = 100, device="gpu")
# G = alpha * TotalVariation(max_iteration = 100)
sirt = SIRT(initial = initial, operator = A, data = data2D,
update_objective_interval = 20, constraint = G,
max_iteration = 200)
# sirt.fix_weights()
sirt.run(verbose=1) |
@MargaretDuff I was wrong with the Also, from the above experiments, |
Thanks @epapoutsellis - I will remove the constraint from the update objective in the PR #1658.
Also in the PR #1658 I changed the code so that CIL TotalVariation isn't calculated in place, so it should work and I added a unit test to test it. Will check with warm starting |
Hi Vaggelis, taking a look, I am confused when you say that:
In the warm start example you show above, the solution is Zeros, which is different to FISTA and PDHG? Why do you say it is "more accurate" |
All the solutions for all algorithms should be
0.24999999997841021 0.2499999999784153 do you run with current master? |
Thanks for your help Vaggelis! Have added in the unit test:
|
The update method of SIRT
CIL/Wrappers/Python/cil/optimisation/algorithms/SIRT.py
Lines 193 to 198 in fb95d17
assumes$\frac{1}{2}||Ax-d||^{2}$ but in the objective method the
CIL/Wrappers/Python/cil/optimisation/algorithms/SIRT.py
Line 212 in fb95d17
In addition, if another constraint is passed, i.e.,
TV
this is considered in theupdate
method but not in theupdate_objective
.The text was updated successfully, but these errors were encountered: