sigpy icon indicating copy to clipboard operation
sigpy copied to clipboard

L2Reg error or misleading docs

Open parthe opened this issue 3 years ago • 3 comments

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 avatar Aug 27 '21 22:08 parthe

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()

parthe avatar Aug 27 '21 22:08 parthe

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 avatar Aug 29 '21 19:08 sidward

I created a pull request

parthe avatar Sep 08 '21 16:09 parthe