TomographicImaging / CIL

A versatile python framework for tomographic imaging
https://tomographicimaging.github.io/CIL/
Apache License 2.0
95 stars 41 forks source link

L1Norm().proximal() is incorrect for complex-valued data #1489

Closed fmwatson closed 10 months ago

fmwatson commented 1 year ago

The current implementation of L1Norm().proximal will always return a real-valued array, because the implementation of soft_shrinkage(x,tau) is not valid for complex x.

the last line should be: out *= numpy.exp(1j*numpy.angle(x))

(this is the same as x.sign() for x real, up to data type)

I'm reminded I need to submit a pull request for proximal of some functions applied to the magnitude image (which would provide the fix), although I think L1Norm().proximal() ought to be correct itself for complex x since || numpy.abs(x) ||_1 = || x ||_1 for complex valued x.

jakobsj commented 1 year ago

Many thanks for submitting this! Would you be up for - in the first instance - submitting a small pull request to address just this? That would be wonderful!

On Wed, 26 Jul 2023, 16.51 fmwatson, @.***> wrote:

The current implementation of L1Norm().proximal will always return a real-valued array, because the implementation of soft_shrinkage(x,tau) is not valid for complex x.

the last line should be: out = numpy.exp(1jnumpy.angle(x))

(this is the same as x.sign() for x real, up to data type)

I'm reminded I need to submit a pull request for proximal of some functions applied to the magnitude image (which would provide the fix), although I think L1Norm().proximal() ought to be correct itself for complex x since || numpy.abs(x) ||_1 = || x ||_1 for complex valued x.

— Reply to this email directly, view it on GitHub https://github.com/TomographicImaging/CIL/issues/1489, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACMDSCC4XR7MEXDNUWP5O3TXSEVGNANCNFSM6AAAAAA2YW3C7A . You are receiving this because you are subscribed to this thread.Message ID: @.***>

fmwatson commented 1 year ago

@jakobsj I can do that when I get a minute!

paskino commented 1 year ago

It would be useful to think about a unit test too, please