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

Rename to avoid overwriting in loop #412

Open
wants to merge 1 commit into
base: development
Choose a base branch
from

Conversation

SJaffa
Copy link
Contributor

@SJaffa SJaffa commented Jul 4, 2023

Closes #378

I'm not sure initial_res is the most sensible name here but @krishnakumarg1984 it does build and pass all the tests.
@mmcleod89 you read the papers most recently, do you have any idea what this res variable is and what a sensible name might be?

@mmcleod89
Copy link
Contributor

I am struggling to figure out what precisely res is... usually it means result or residual, and it doesn't appear to be a residual. We use Psi.adjoint to convert from the image domain to the wavelet domain, apply some threshold and scaling, and then convert back to the image domain and subtract it from x so it appears to be some kind of intermediate value that is used in the udate, but I can't quite figure out how it relates to the papers right now - it doesn't help that the use of nu, gamma, lambda, x, z etc. is very inconsistent!

Another question that I think this change raises is why are we making these const variables in the first place? What is the point of keeping initial_res given that it's never used again? This is an iterative method, and the initial value is just declared outside the loop. Wouldn't it make more sense to declare it non-const and just update it every iteration and re-use the memory rather than creating and destroying the vector every iteration? (Granted it might be optimised out, but I still don't see why we wouldn't just use a non const variable to begin with.)

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.

Code quality: Shadow variable declaration in l1_proximal.h
2 participants