Skip to content

WIP real-valued functional #1333

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.

Sign up for GitHub

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

Open
wants to merge 17 commits into
base: master
Choose a base branch
from
Open

WIP real-valued functional #1333

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
d4e5498
Tried to make functional possible to map to real numbers istead of sp…
Apr 26, 2018
7a278f0
Merge branch 'master' of https://github.com/miallo/odl
Apr 26, 2018
09b1e94
Continued implementation of functionals mapping to RealNumbers
Apr 26, 2018
89652b1
Added range to functional operations
Apr 27, 2018
69521ba
Continued to implement range in default_functionals
Apr 27, 2018
85650f7
Mostly continued default_fuctionals
Apr 27, 2018
7382635
fixed mistake where renaming proximal_l* function inputs to `domain`
Apr 27, 2018
246a31b
fixed range of QuadraticForm-Functional
Apr 27, 2018
5a55e75
Implemented most comments from @kohr-h and @aringh
Apr 27, 2018
d1c4d68
Is there a reason for ufunc_ops to pass the explicit keyword `domain=…
Apr 27, 2018
9003572
Added docstrings for `range` parameter
Apr 27, 2018
77842df
Fields can now be added to the "highest" one
Apr 29, 2018
c7a6127
Removed addition of fields again, Reverted `__repr__` not to include …
Apr 29, 2018
3fa93d9
Changed the order to (domain, range, ...) in __init__ calls and fixed
May 31, 2018
0951041
fixed typo in forward-backward documentation
Jun 5, 2018
0ddd06c
Merged from odl main branch
Jun 13, 2018
cb6df08
CallbackShowConvergence now supports saveto
Jun 14, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions odl/operator/default_ops.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ class LinCombOperator(Operator):
LinCombOperator(a, b)([x, y]) == a * x + b * y
"""

def __init__(self, space, a, b):
def __init__(self, space, a, b, rangeType=None):
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, that wasn't meant to be pushed... I used it for a mapping-oparator from R2 to C...

"""Initialize a new instance.

Parameters
Expand All @@ -204,8 +204,11 @@ def __init__(self, space, a, b):
>>> z
rn(3).element([ 2., 4., 6.])
"""
if rangeType is None:
rangeType = space.dtype
domain = ProductSpace(space, space)
super(LinCombOperator, self).__init__(domain, space, linear=True)
super(LinCombOperator, self).__init__(domain, space.astype(rangeType),
linear=True)
self.a = a
self.b = b

Expand Down
13 changes: 9 additions & 4 deletions odl/operator/operator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1076,7 +1076,6 @@ class OperatorSum(Operator):

The sum is only well-defined for `Operator` instances where
`Operator.range` is a `LinearSpace`.

"""

def __init__(self, left, right, tmp_ran=None, tmp_dom=None):
Expand Down Expand Up @@ -1111,8 +1110,14 @@ def __init__(self, left, right, tmp_ran=None, tmp_dom=None):
rn(3).element([ 2., 4., 6.])
"""
if left.range != right.range:
raise OpTypeError('operator ranges {!r} and {!r} do not match'
''.format(left.range, right.range))
if (isinstance(left.range, Field) and
isinstance(right.range, Field)):
range = left.range + right.range
else:
raise OpTypeError('operator ranges {!r} and {!r} do not match'
''.format(left.range, right.range))
else:
range = left.range
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I'd prefer not to have any clever mechanism for "fixing" cases that "obviously" should work, like

L1Norm(cn(2), range=ComplexNumbers()) + L1Norm(cn(2), range=RealNumbers())

IMO this should fail, and users should explicitly have to use RealPart or ComplexEmbedding to state their intentions (of course, for that, those operators need to work with RealNumbers and ComplexNumbers, resp.).

if not isinstance(left.range, (LinearSpace, Field)):
raise OpTypeError('`left.range` {!r} not a `LinearSpace` or '
'`Field` instance'.format(left.range))
Expand All @@ -1129,7 +1134,7 @@ def __init__(self, left, right, tmp_ran=None, tmp_dom=None):
''.format(tmp_dom, left.domain))

super(OperatorSum, self).__init__(
left.domain, left.range,
left.domain, range,
linear=left.is_linear and right.is_linear)
self.__left = left
self.__right = right
Expand Down
6 changes: 3 additions & 3 deletions odl/set/sets.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -428,10 +428,10 @@ def __hash__(self):

def element(self, inp=None):
"""Return a real number from ``inp`` or from scratch."""
if inp is not None:
return float(inp)
else:
if inp is None:
return 0.0
else:
return float(getattr(inp, 'real', inp))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Besides the implementation, I'm wondering if we really want this implicit conversion. I'd actually prefer to let this fail for objects that don't implement __float__ but do implement real.
There's also a weird side effect for objects that implement both __float__ and real, namely that real is preferred over __float__. That doesn't seem quite right to me.
Other opinions, @aringh, @adler-j?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just needed something to convert the complex numbers to real ones. You could instead have elif dtype(inp)==complex: return inp.real; else: return float(inp). Or do you have a different idea?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, my point was that IMO this shouldn't happen in the first place. If you call RealNumbers().element(1 + 1j) it should fail rather than taking the real part.

To me the better approach would be to make the RealPart and ImagPart operators (and their friends) work for RealNumbers and ComplexNumbers, respectively. Likely this requires little less than implementing .real and .imag on those classes, but that's not a guarantee 😉

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this last case, should ImagPart(RealNumbers().element(1)) give 0 or should it fail? The latter represents seeing RealNumbers and ComplexNumbers as two different entities, while the former somehow implicitly assumes that the real numbers are seen as embedded in the complex numbers, just with imaginary part 0. Which one is preferred?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd take Python (and NumPy) as a guide here:

>>> x = 1.0
>>> x.imag
0.0
>>> x = np.array(1.0)
>>> x.imag
array(0.)
>>> float(1 + 1j)
...
TypeError: can't convert complex to float
>>> np.array(1 + 1j, dtype=float)
...
TypeError: can't convert complex to float

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some quick trials gives:

>>> (1).imag
0

Hence, ImagPart(RealNumbers().element(1)) should return 0.


@property
def examples(self):
Expand Down
Loading