Skip to content
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

Add solver sporco #19

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open

Add solver sporco #19

wants to merge 16 commits into from

Conversation

EnLAI111
Copy link
Collaborator

No description provided.

@EnLAI111
Copy link
Collaborator Author

EnLAI111 commented Aug 1, 2022

The test can't be passed because sporco only works for isotropic cases, but the default parameter of objective is anisotropic. And I can't simply change the default isotropy, because proxtv only works for anisotropic cases.

I think of adding test_parameters to objective as follows.

test_parameters = {
    'isotropy': ["anisotropic", "isotropic"]
}

However, I have checked codes in benchopt, it seems like test_parameters works only for dataset.
So maybe I can open an issue in benchopt about this?

@EnLAI111
Copy link
Collaborator Author

EnLAI111 commented Aug 1, 2022

The solver sporco works for denoising but it does not converge to the same point as other solvers do in deblurring cases.

image

image

@agramfort
Copy link
Contributor

hi @bwohlberg !

with a student we are writing a benchmark using https://benchopt.github.io on TV problems.
We are adding Sporco as a solver here. Is there a chance you can have look if we are doing something wrong in the deblurring case? 🙏

@bwohlberg
Copy link

Sure, happy to try to help. Are you referring to the test failure in CI, or some other specific problem?

@EnLAI111
Copy link
Collaborator Author

EnLAI111 commented Aug 3, 2022

Sure, happy to try to help. Are you referring to the test failure in CI, or some other specific problem?

Thx @bwohlberg ! I think the test failure is not caused by the setting for sporco, I can briefly explain my problem.

We are trying to build a regularised TV-2D benchmark,
$$\boldsymbol{u} \in \underset{\boldsymbol{u} \in \mathbb{R}^{n \times m}}{\mathrm{argmin}} f(\boldsymbol{y}, A \boldsymbol{u}) + \lambda \| \boldsymbol{u} \|_{TV}$$

Since here $f$ can be quadratic loss or Huber loss, and the operator of convolution $A$'s type is LinearOperator not array, I can not directly use sporco.admm.tvl2.TVL2Deconv.

So, I wonder if I can use sporco.admm.tvl2.TVL2Denoise to do a gradient descent like
$$\boldsymbol{u}^{(t+1)} = \mathrm{sporco}_{\lambda\gamma} (\boldsymbol{u}^{(t)} - \gamma \triangledown_{\boldsymbol{u}} f(\boldsymbol{y}, A \boldsymbol{u}^{(t)}))$$
where $\gamma$ is a step size.

Now this seems work in the denoising case (it converges to the same point as other solvers do), but not work in the deblurring case, which are shown in the following figures.

image

image

So I don't know the problem is that I can not use this method, or there is something wrong with the opt.

opt = tvl2.TVL2Denoise.Options({'FastSolve': True, 'Verbose': False,
                                                      'MaxMainIter': 100, 'AbsStopTol': 0.,
                                                      'RelStopTol': 0., 'RelaxParam': 0.5,
                                                      'AutoRho': {'AutoScaling': False, 'Enabled': False},
                                                      'gEvalY': False, 'MaxGSIter': 100, 'GSTol': 0.})

@bwohlberg
Copy link

I don't see anything obviously wrong with the opt, but performance of nested iterations is often quite sensitive to stopping criteria for the inner problem, so it may be playing a role. Would you be able to create a "minimal example" script with just the scico implementation so that I can do some experimentation to try to figure out what's going wrong?

Some higher-level comments:

Your approach seems reasonable from a mathematical perspective: you're using the sporco TV denoiser as a prox for the TV norm. I assume you're using PGM at the outer level because it's easier to support choices of $f$ that don't correspond to a squared $\ell_2$ norm (for which I would expect ADMM at the outer level to be more efficient)?

It's not clear, though, how meaningful this is from a benchmarking perspective, at least in terms of time performance, since I expect that the nested iterations are going to make this approach very much slower than some of the other methods. When the linear operator $A$ is chosen as a circular convolution, the dedicated sporco code for this situation will be vastly faster. Whether the approach you're taking makes sense really depends on the ultimate goals of the benchmarking, but the results should not be taken as being generically representative of the performance of ADMM for these problems.

@EnLAI111
Copy link
Collaborator Author

EnLAI111 commented Aug 4, 2022

@bwohlberg Thank your so much!
I am not sure I have understood what you do mean by "minimal example", do you want an example including only sporco as solver for benchopt? If so, I have created a branch called mini_example, you can follow the steps in README.rst to make it work.

@bwohlberg
Copy link

Sorry for not being sufficiently clear. It would really be helpful if you could create a stand-alone script, independent of the benchmark framework, that creates the test data and runs just the sporco solver. If that's not feasible, then I'll try your mini-example branch, but it may take more time to figure out what's going on with the additional benchmark infrastructure included.

@bwohlberg
Copy link

By the way, from a quick scan of the code setting up the TV solver, I can't see any sign of the ADMM penalty parameter (option "Rho", if I recall correctly) being set, which would imply it's just using the default value. For now, this would be my best guess as to the cause of the poor convergence.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants