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

Bug in prox of l1 norm #1181

Closed
mehrhardt opened this issue Oct 9, 2017 · 9 comments
Closed

Bug in prox of l1 norm #1181

mehrhardt opened this issue Oct 9, 2017 · 9 comments

Comments

@mehrhardt
Copy link
Contributor

This example shows a bug in the prox operator of the l1 norm:

import odl

X = odl.rn(1)
f = odl.solvers.L1Norm(X)
x = 0.1 * X.one()

y1 = x.copy()
f.convex_conj.proximal(3)(y1, out=y1)

y2 = x.copy()
y2 = f.convex_conj.proximal(1)(y2)

(y1-y2).norm()

results in 0.9. Similar effects can be observed in KullbackLeibler but not in L2Norm.

@adler-j
Copy link
Member

adler-j commented Oct 9, 2017

What would be the expected result and why?

Edit: I'm stupid, ofc it should be zero.

@adler-j adler-j closed this as completed in 9c9002d Oct 9, 2017
adler-j added a commit that referenced this issue Oct 9, 2017
BUG: Fix bug with aliased input to l1_cconj proximal, closes #1181
@mehrhardt
Copy link
Contributor Author

As mentioned above already, replacing f by f = odl.solvers.KullbackLeibler(X) yields similar issues. Who knows which other functionals have the same problem.

@adler-j
Copy link
Member

adler-j commented Oct 9, 2017

Darn it, you are right. I'll look into it.

@mehrhardt
Copy link
Contributor Author

Any news here? Is there a way to write a test that gets run on all functionals (without us writing a new one every time)?

@adler-j
Copy link
Member

adler-j commented Oct 13, 2017

No update sadly, havn't had time. With that said there is a rather enormous large-scale test here:

def test_proximal_defintion(functional, stepsize):

it should catch stuff like this, but apparently does not.

@kohr-h
Copy link
Member

kohr-h commented Oct 22, 2017

No update sadly, havn't had time. With that said there is a rather enormous large-scale test here:

odl/odl/test/largescale/solvers/nonsmooth/default_functionals_slow_test.py

Line 114 in baecb8a
def test_proximal_defintion(functional, stepsize):

it should catch stuff like this, but apparently does not.

Does it test with aligned input & output?

@adler-j
Copy link
Member

adler-j commented Oct 22, 2017

Does it test with aligned input & output?

Very much looks like it does not.

kohr-h pushed a commit to kohr-h/odl that referenced this issue Nov 10, 2017
@kohr-h kohr-h closed this as completed in 14d2ab5 Nov 11, 2017
kohr-h pushed a commit that referenced this issue Nov 11, 2017
BUG: fix in-out aliasing issue in OperatorSum, closes #1181
@mehrhardt
Copy link
Contributor Author

Well done! This indeed fixed the bug I was observing.

@mehrhardt
Copy link
Contributor Author

mehrhardt commented Apr 27, 2021

I just did a similar test with the l1-norm itself and not its conjugate. While the latter is indeed fixed, the former still has the same problem:

import odl

X = odl.rn(1)
f = odl.solvers.L1Norm(X)
x = 0.1 * X.one()

y1 = x.copy()
f.proximal(1e-2)(y1, out=y1)

y2 = x.copy()
y2 = f.proximal(1e-2)(y2)

(y1-y2).norm()

@adler-j , @kohr-h , any chance this can be fixed easily?

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

3 participants