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

L2Reg error or misleading docs #94

Open
parthe opened this issue Aug 27, 2021 · 3 comments
Open

L2Reg error or misleading docs #94

parthe opened this issue Aug 27, 2021 · 3 comments
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@parthe
Copy link

parthe commented Aug 27, 2021

In https://sigpy.readthedocs.io/en/latest/generated/sigpy.prox.L2Reg.html#sigpy.prox.L2Reg

is the proximal operator a function of y or z. It appears that the base class sigpy.prox.Prox uses y as the input to the prox operator. But y is a parameter for the L2Reg class called "bias".

I think "z" is the bias based on what the description is. My tests confirm this.

See below for a test which passes based on the assumption that z is the bias parameter and y is the input to the prox operator.

Also the statement in the docs above says min instead of argmin.

@parthe parthe added the bug Something isn't working label Aug 27, 2021
@parthe
Copy link
Author

parthe commented Aug 27, 2021

import numpy as np
from sigpy.prox import L2Reg
import unittest

def g_in(r_in_minus, gamma_in_minus):
    eta_in_plus = (1 + gamma_in_minus)
    xhat_in_plus = r_in_minus * gamma_in_minus / eta_in_plus
    return xhat_in_plus, eta_in_plus

class TestingDenoisers(unittest.TestCase):

    def test_g_in(self):
        N=10
        operator = L2Reg((N,1), 1.)
        r = np.random.randn(N,1)
        gamma = abs(np.random.randn())
        xhat, eta = g_in(r, gamma)
        C = np.cov(r, xhat, rowvar=False)
        eta_check = gamma/(C[1,0]/C[0,0])
        self.assertAlmostEqual(eta_check, eta)
        self.assertAlmostEqual(np.linalg.norm(xhat-operator(1/gamma, r)),0.)

if __name__ == '__main__':
    unittest.main()

@sidward
Copy link
Collaborator

sidward commented Aug 29, 2021

Hello! Thanks for the report. Are you open to making a pull request for your requested document changes? In particular, if you're able to change the documentation for all the "basic proxs" listed in https://sigpy.readthedocs.io/en/latest/core_prox.html to be of the format "prox_{alpha g)(y) = argmin...", that would be much appreciated.

@sidward sidward added the help wanted Extra attention is needed label Sep 2, 2021
@parthe
Copy link
Author

parthe commented Sep 8, 2021

I created a pull request

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants