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

Primal Dual testcase in algo_factory test does not converge to desired accuracy #317

Open
1 task
tkoskela opened this issue Jun 16, 2023 · 2 comments
Open
1 task

Comments

@tkoskela
Copy link
Contributor

The test for the primal dual algorithm in sopt

https://github.com/astro-informatics/sopt/blob/ae4c48e397b99d6b27b727744da292cb3f1817ce/cpp/tests/primal_dual.cc#L19

is checking that the algorithm converges on a random vector of size 5. As far as we understand it, this checks that the algorithm does not crash, but does not really test the convergence behaviour on a real image. The test in purify

TEST_CASE("primal_dual_factory") {

is testing the algorithm against a 128x128 image. The purify test is failing in #315, the solution goes to zero and if x is 0 then Lx is also 0 and the solution can never get a non-zero value. We don't currently understand whether this is a problem with the algorithm, or the implementation.

This is a low priority to fix, we can merge #315 and #302 while the test is failing. This should be further investigated in the medium term. It would probably make most sense to fix this together with with astro-informatics/sopt#356

We know that the test was passing at the beginning of our github actions history but the log files are gone. That would however indicate that the algorithm is not fundamentally broken, but rather we have accidentally changed something.

TODO list:

  • Swap the algorithm in the inpainting test in sopt from ForwardBackward to PrimalDual, and see if it converges on a non-zero solution.
@tkoskela
Copy link
Contributor Author

tkoskela commented Sep 5, 2024

We've abandoned the primal dual algorithm and the test has been set to xfail.

@jasonmcewen
Copy link
Contributor

@tkoskela Reopening this issue as we cannot abandon the primal dual algorithm. While we're been working on alternatives recently this is the prior state-of-the-art approach and we need to still have this working.

@jasonmcewen jasonmcewen reopened this Sep 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants