WIP real-valued functional

[miallo]

I started working on the implementation of the range of Functional as mentioned in #1328.

If I am not mistaken the old implementation of FunctionalRightVectorMult leads to the functional “forgetting” its grad_lipschitz and linear properties.

I tagged some places in functional.py with “# HELP” where I am especially unsure about what I am doing.

I did NOT implement any kind of typecasting anywhere yet but just tried to make sure that the range property gets passed along.

[pep8speaks]

Checking updated PR...

No PEP8 issues.

Comment last updated on June 14, 2018 at 22:12 Hours UTC

[kohr-h]

Regarding checks, general remark:

• If __init__ inputs are simply passed on to other __init__ methods, the checks done there are sufficient, unless a subclass needs its inputs to satisfy additional conditions.

  def __init__(self, domain, range):
      super(...).__init__(domain, range)   # OK, parent class does the checking
  

• If before passing them to another __init__, inputs are compared to other things with is or == it's also okay not to check types and let the __eq__ implementation handle it.

  def __init__(self, domain, range=None):
      if domain == RealNumbers():  # OK
          # do stuff
      if range is None:  # OK
          # do other stuff
      super(...).__init__(domain, range)
  

• If before passing them to another __init__, attributes of the inputs are used, we usually check beforehand to avoid errors like 'Foo' has no attribute 'bar':

  def __init__(self, domain, range=None):
      if range is None:
          # not great, if domain has no `field` attribute, error message is bad
          range = domain.field
      if not isinstance(domain, LinearSpace):
          # also not great, too restrictive (excludes `RealNumbers` for instance)
          raise ...
      # Better: duck-typing
      default_range = getattr(domain, 'field', RealNumbers())  # version 1, default RealNumbers
      default_range = getattr(domain, 'field', None)  # version 2, no default
      if range is None:
          if default_range is None:  # for variant 2 where there's no default
              raise ...
          range = default_range
  
      super(...).__init__(domain, range)
  

[kohr-h]

Merged from odl main branch

Two things:

1. We don't do merge from master, please rebase instead. That keeps a nice and clean history. (If you need to know how it works, just ask.)
2. We have some guidelines regarding commit messages that we try to follow ourselves (though sometimes we get sloppy, too :wink:). It makes the list of commits much more digestible if you see at a glance which commit does what. Short version:
   • Prefix with BUG:, MAINT:, STY:, TST: etc.
   • Use imperative style ("Fix bug", not "Fixed bug")
   • Fit first line in ~50 characters, then go wild after a blank line

I guess this PR is gonna end up as one big squashed commit since it's been going a bit back and forth (no issue with that), so we'll likely fix it then.

[adler-j]

So what's the status of this? I'd love for this to get finalized!

[miallo]

So what's the status of this?

As I mentioned somewhere before: I have never written any tests in python and I would rather have someone else do them since I am not 100% confident with what is to be expected for the default values (expecially for things like IndicatorLpUnitBall.convex_conj). Otherwise I hope I found most of the 'obvious' bugs for now...

[adler-j]

I'm currently very busy leading up to the vacation, but I can do that after summer. Thanks again for the contribution!