Skip to content

Commit 357c1fc

Browse files
embrayeteq
authored andcommitted
Merge pull request #4236 from embray/modeling/inverse-update
Model.inverse tweaks
1 parent 65bc14d commit 357c1fc

7 files changed

Lines changed: 201 additions & 46 deletions

File tree

‎CHANGES.rst‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -392,6 +392,19 @@ API changes
392392
with True only if the comparison was true for all elements compared,
393393
which could lead to confusing circumstances. [#3912]
394394

395+
- Using ``model.inverse = None`` to reset a model's inverse to its
396+
default is deprecated. In the future this syntax will explicitly make
397+
a model not have an inverse (even if it has a default). Instead, use
398+
``del model.inverse`` to reset a model's inverse to its default (if it
399+
has a default, otherwise this just deletes any custom inverse that has
400+
been assigned to the model and is still equivalent to setting
401+
``model.inverse = None``). [#4236]
402+
403+
- Adds a ``model.has_user_inverse`` attribute which indicates whether or not
404+
a user has assigned a custom inverse to ``model.inverse``. This is just
405+
for informational purposes, for example, for software that introspects
406+
model objects. [#4236]
407+
395408
- Renamed the parameters of ``RotateNative2Celestial`` and
396409
``RotateCelestial2Native`` from ``phi``, ``theta``, ``psi`` to
397410
``lon``, ``lat`` and ``lon_pole``. [#3578]

‎astropy/modeling/core.py‎

Lines changed: 73 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -246,33 +246,22 @@ def rename(cls, name):
246246

247247
def _create_inverse_property(cls, members):
248248
inverse = members.get('inverse')
249-
if inverse is None:
249+
if inverse is None or cls.__bases__[0] is object:
250+
# The latter clause is the prevent the below code from running on
251+
# the Model base class, which implements the default getter and
252+
# setter for .inverse
250253
return
251254

252255
if isinstance(inverse, property):
253-
fget = inverse.fget
254-
else:
255256
# We allow the @property decorator to be omitted entirely from
256257
# the class definition, though its use should be encouraged for
257258
# clarity
258-
fget = inverse
259-
260-
def wrapped_fget(self):
261-
if self._custom_inverse is not None:
262-
return self._custom_inverse
263-
264-
return fget(self)
265-
266-
def fset(self, value):
267-
if not isinstance(value, (Model, type(None))):
268-
raise ValueError(
269-
"The ``inverse`` attribute may be assigned a `Model` "
270-
"instance or `None` (where `None` restores the default "
271-
"inverse for this model if one is defined.")
272-
273-
self._custom_inverse = value
259+
inverse = inverse.fget
274260

275-
cls.inverse = property(wrapped_fget, fset, doc=inverse.__doc__)
261+
# Store the inverse getter internally, then delete the given .inverse
262+
# attribute so that cls.inverse resolves to Model.inverse instead
263+
cls._inverse = inverse
264+
del cls.inverse
276265

277266
def _handle_backwards_compat(cls, name, members):
278267
# Backwards compatibility check for 'eval' -> 'evaluate'
@@ -579,7 +568,8 @@ class Model(object):
579568
# optionally; in that case it is of course up to the user to determine
580569
# whether their inverse is *actually* an inverse to the model they assign
581570
# it to.
582-
_custom_inverse = None
571+
_inverse = None
572+
_user_inverse = None
583573

584574
# If a bounding_box_default function is defined in the model,
585575
# then the _bounding_box attribute should be set to 'auto' in the model.
@@ -777,8 +767,6 @@ def ineqcons(self):
777767

778768
return self._constraints['ineqcons']
779769

780-
# *** Public methods ***
781-
782770
@property
783771
def inverse(self):
784772
"""
@@ -791,16 +779,73 @@ def inverse(self):
791779
`~astropy.modeling.polynomial.PolynomialModel`, but not by
792780
requirement).
793781
782+
A custom inverse can be deleted with ``del model.inverse``. In this
783+
case the model's inverse is reset to its default, if a default exists
784+
(otherwise the default is to raise `NotImplementedError`).
785+
794786
Note to authors of `~astropy.modeling.Model` subclasses: To define an
795787
inverse for a model simply override this property to return the
796788
appropriate model representing the inverse. The machinery that will
797789
make the inverse manually-overridable is added automatically by the
798790
base class.
799791
"""
800792

793+
if self._user_inverse is not None:
794+
return self._user_inverse
795+
elif self._inverse is not None:
796+
return self._inverse()
797+
801798
raise NotImplementedError("An analytical inverse transform has not "
802799
"been implemented for this model.")
803800

801+
@inverse.setter
802+
def inverse(self, value):
803+
if not isinstance(value, (Model, type(None))):
804+
raise ValueError(
805+
"The ``inverse`` attribute may be assigned a `Model` "
806+
"instance or `None` (where `None` restores the default "
807+
"inverse for this model if one is defined.")
808+
809+
if value is None:
810+
warnings.warn(
811+
"Currently setting `model.inverse = None` resets the inverse "
812+
"to the default inverse (if one exists). However, starting "
813+
"in Astropy 1.2, setting `model.inverse = None` explicitly "
814+
"forces a model to have no inverse (such that accessing "
815+
"`model.inverse` raises a NotImplementedError) even if that "
816+
"model's class has a default inverse.\n\n"
817+
"Instead, call `del model.inverse` to reset the inverse to "
818+
"its default (if a default exists for that model's class--"
819+
"otherwise the model is reset to having no inverse.",
820+
AstropyDeprecationWarning)
821+
822+
self._user_inverse = value
823+
824+
@inverse.deleter
825+
def inverse(self):
826+
"""
827+
Resets the model's inverse to its default (if one exists, otherwise
828+
the model will have no inverse).
829+
"""
830+
831+
del self._user_inverse
832+
833+
@property
834+
def has_user_inverse(self):
835+
"""
836+
A flag indicating whether or not a custom inverse model has been
837+
assigned to this model by a user, via assignment to ``model.inverse``.
838+
"""
839+
840+
return self._user_inverse is not None
841+
842+
@property
843+
def _custom_inverse(self):
844+
# Deprecated alias for _user_inverse--included solely for temporary
845+
# backwards compatibility for pyasdf--remove after Astropy v1.1
846+
# release.
847+
return self._user_inverse
848+
804849
@property
805850
def bounding_box(self):
806851
"""
@@ -889,6 +934,8 @@ def bounding_box(self, limits):
889934

890935
self._bounding_box = limits
891936

937+
# *** Public methods ***
938+
892939
@abc.abstractmethod
893940
def evaluate(self, *args, **kwargs):
894941
"""Evaluate the model on some input variables."""
@@ -1832,7 +1879,7 @@ class members that should be added to the generated
18321879
# intermediate compound models are stored in the tree as well then
18331880
# we can immediately check for custom inverses on sub-models when
18341881
# computing the inverse
1835-
instance._custom_inverse = mcls._make_custom_inverse(
1882+
instance._user_inverse = mcls._make_user_inverse(
18361883
operator, left, right)
18371884

18381885
return instance
@@ -1851,7 +1898,7 @@ def _handle_backwards_compat(mcls, name, members):
18511898
return
18521899

18531900
@classmethod
1854-
def _make_custom_inverse(mcls, operator, left, right):
1901+
def _make_user_inverse(mcls, operator, left, right):
18551902
"""
18561903
Generates an inverse `Model` for this `_CompoundModel` when either
18571904
model in the operation has a *custom inverse* that was manually
@@ -1873,7 +1920,7 @@ def _make_custom_inverse(mcls, operator, left, right):
18731920
"""
18741921

18751922
if not (operator in ('&', '|') and
1876-
(left._custom_inverse or right._custom_inverse)):
1923+
(left._user_inverse or right._user_inverse)):
18771924
# These are the only operators that support an inverse right now
18781925
return None
18791926

‎astropy/modeling/functional_models.py‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -688,6 +688,12 @@ def fit_deriv(x, slope, intercept):
688688
d_intercept = np.ones_like(x)
689689
return [d_slope, d_intercept]
690690

691+
@property
692+
def inverse(self):
693+
new_slope = self.slope ** -1
694+
new_intercept = -self.intercept / self.slope
695+
return self.__class__(slope=new_slope, intercept=new_intercept)
696+
691697

692698
class Lorentz1D(Fittable1DModel):
693699
"""

‎astropy/modeling/tests/test_core.py‎

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,15 @@
33
from __future__ import (absolute_import, unicode_literals, division,
44
print_function)
55

6-
import pytest
76
import numpy as np
87
from numpy.testing.utils import assert_allclose
98
from ..core import Model, InputParameterError, custom_model, render_model
109
from ..parameters import Parameter
1110
from .. import models
1211

12+
from ...tests.helper import pytest, catch_warnings
1313
from ...utils.compat.funcsigs import signature
14+
from ...utils.exceptions import AstropyDeprecationWarning
1415

1516

1617
class NonFittableModel(Model):
@@ -200,12 +201,43 @@ def test_custom_inverse():
200201
assert_allclose(x, p(p.inverse(x)))
201202
assert_allclose(x, p.inverse(p(x)))
202203

203-
p.inverse = None
204+
with catch_warnings(AstropyDeprecationWarning) as w:
205+
p.inverse = None
206+
207+
# TODO: This can be removed after Astropy v1.1 or so
208+
assert len(w) == 1
204209

205210
with pytest.raises(NotImplementedError):
206211
p.inverse
207212

208213

214+
def test_custom_inverse_reset():
215+
"""Test resetting a custom inverse to the model's default inverse."""
216+
217+
class TestModel(Model):
218+
inputs = ()
219+
outputs = ('y',)
220+
@property
221+
def inverse(self):
222+
return models.Shift()
223+
224+
@staticmethod
225+
def evaluate():
226+
return 0
227+
228+
# The above test model has no meaning, nor does its inverse--this just
229+
# tests that setting an inverse and resetting to the default inverse works
230+
231+
m = TestModel()
232+
assert isinstance(m.inverse, models.Shift)
233+
234+
m.inverse = models.Scale()
235+
assert isinstance(m.inverse, models.Scale)
236+
237+
del m.inverse
238+
assert isinstance(m.inverse, models.Shift)
239+
240+
209241
def test_render_model_2d():
210242

211243
imshape = (71, 141)

‎docs/modeling/models.rst‎

Lines changed: 69 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,19 @@ a model may be instantiated with all scalar parameters::
3030
>>> g
3131
<Gaussian1D(amplitude=1.0, mean=0.0, stddev=1.0)>
3232

33+
The newly created model instance ``g`` now works like a Gaussian function
34+
with the given parameters fixed. It takes a single input::
35+
36+
>>> g.inputs
37+
('x',)
38+
>>> g(x=0)
39+
1.0
40+
41+
The model can also be called without explicitly using keyword arguments::
42+
43+
>>> g(0)
44+
1.0
45+
3346
Or it may use all array parameters. For example if all parameters are 2x2
3447
arrays the model is computed element-wise using all elements in the arrays::
3548

@@ -181,24 +194,62 @@ set. For scalar inputs like ``g(0)``, ``model_set_axis=False`` is implied
181194
automatically. But for array inputs it is necessary to avoid ambiguity.
182195

183196

184-
Inputs and Outputs
185-
==================
186-
187-
Models have an `~astropy.modeling.Model.n_inputs` attribute, which shows how
188-
many coordinates the model expects as an input. All models expect coordinates
189-
as separate arguments. For example a 2-D model expects x and y coordinate
190-
values to be passed separately, i.e. as two scalars or array-like values.
191-
192-
Models also have an attribute `~astropy.modeling.Model.n_outputs`, which shows
193-
the number of output coordinates. The `~astropy.modeling.Model.n_inputs` and
194-
`~astropy.modeling.Model.n_outputs` attributes can be used when chaining
195-
transforms by adding models in :class:`series
196-
<astropy.modeling.SerialCompositeModel>` or in :class:`parallel
197-
<astropy.modeling.SummedCompositeModel>`. Because composite models can be
198-
nested within other composite models, creating theoretically infinitely complex
199-
models, a mechanism to map input data to models is needed. In this case the
200-
input may be wrapped in a `~astropy.modeling.LabeledInput` object-- a dict-like
201-
object whose items are ``{label: data}`` pairs.
197+
Model Inverses
198+
==============
199+
200+
All models have a `Model.inverse <astropy.modeling.Model.inverse>` property
201+
which may, for some models, return a new model that is the analytic inverse of
202+
the model it is attached to. For example::
203+
204+
>>> from astropy.modeling.models import Linear1D
205+
>>> linear = Linear1D(slope=0.8, intercept=1.0)
206+
>>> linear.inverse
207+
<Linear1D(slope=1.25, intercept=-1.25)>
208+
209+
The inverse of a model will always be a fully instantiated model in its own
210+
right, and so can be evaluated directly like::
211+
212+
>>> linear.inverse(2.0)
213+
1.25
214+
215+
It is also possible to assign a *custom* inverse to a model. This may be
216+
useful, for example, in cases where a model does not have an analytic inverse,
217+
but may have an approximate inverse that was computed numerically and is
218+
represented by a polynomial. This works even if the target model has a
219+
default analytic inverse--in this case the default is overridden with the
220+
custom inverse::
221+
222+
>>> from astropy.modeling.models import Polynomial1D
223+
>>> linear.inverse = Polynomial1D(degree=1, c0=-1.25, c1=1.25)
224+
>>> linear.inverse
225+
<Polynomial1D(1, c0=-1.25, c1=1.25)>
226+
227+
If a custom inverse has been assigned to a model, it can be deleted with
228+
``del model.inverse``. This resets the inverse to its default (if one exists).
229+
If a default does not exist, accessing ``model.inverse`` raises a
230+
`NotImplementedError`. For example polynomial models do not have a default
231+
inverse::
232+
233+
>>> del linear.inverse
234+
>>> linear.inverse
235+
<Linear1D(slope=1.25, intercept=-1.25)>
236+
>>> p = Polynomial1D(degree=2, c0=1.0, c1=2.0, c2=3.0)
237+
>>> p.inverse
238+
Traceback (most recent call last):
239+
File "<stdin>", line 1, in <module>
240+
File "astropy\modeling\core.py", line 796, in inverse
241+
raise NotImplementedError("An analytical inverse transform has not "
242+
NotImplementedError: An analytical inverse transform has not been
243+
implemented for this model.
244+
245+
One may certainly compute an inverse and assign it to a polynomial model
246+
though.
247+
248+
.. note::
249+
250+
When assigning a custom inverse to a model no validation is performed to
251+
ensure that it is actually an inverse or even approximate inverse. So
252+
assign custom inverses at your own risk.
202253

203254

204255
Further examples

‎docs/modeling/new.rst‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,11 @@ input``.
268268
new_intercept = -self.intercept / self.slope
269269
return LineModel(slope=new_slope, intercept=new_intercept)
270270
271+
.. note::
272+
273+
The above example is essentially equivalent to the built-in
274+
`~astropy.modeling.functional_models.Linear1D` model.
275+
271276

272277
Defining New Fitter Classes
273278
===========================

‎docs/nitpick-exceptions‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,3 +63,4 @@ py:obj list.remove
6363
py:class classmethod
6464
py:obj RuntimeError
6565
py:obj AttributeError
66+
py:obj NotImplementedError

0 commit comments

Comments
 (0)