-
Notifications
You must be signed in to change notification settings - Fork 15
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
[QoL] Add Identity() as default linear in Sparsity prox #74
Comments
@MorganSchmitz This is a nice idea, but since An alternative would be to include a new linear operator object (e.g. just called If this will work for you I can make a PR for this. |
Ah, I see the issue with just adding a default... I am not sure I understand your suggested workaround though. Do you mean creating a whole new proximity operator that acts the exact same way as If so, couldn't this get confusing for new users, who would not be sure which of the two they should use? I guess another workaround would be to also add a default to weights, just set to 0. That way, a |
@MorganSchmitz Yes, that should work fine, but I think it might be better to make |
If we plan to fix the cost operation, based on CEA-COSMIC/pysap-mri#80 , doesnt that mean we can remove |
@chaithyagr Indeed. I will probably hold off opening a PR for this until we have a clear plan for how we want to handle the cost contribution from proximity operators. |
Yes makes sense.. |
Is your feature request related to a problem? Please describe.
opt.proximity.SparseThreshold requires a
linear
operator as input I think it'd be good to have a default set up.Describe the solution you'd like
opt.linear.Identity
would be the natural choice.Are you planning to submit a Pull Request?
The text was updated successfully, but these errors were encountered: