Allow splitting Celestial axes #467

nden · 2023-08-06T20:31:29Z

Closes #455

This fixes a problem with evaluating the backwards transform when a CompositeFrame contains a CelestialFrame with non-consecutive axes.

codecov · 2023-08-06T20:33:30Z

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.01% 🎉

Comparison is base (89055d4) 87.23% compared to head (3350441) 87.24%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #467      +/-   ##
==========================================
+ Coverage   87.23%   87.24%   +0.01%     
==========================================
  Files          23       23              
  Lines        3815     3818       +3     
==========================================
+ Hits         3328     3331       +3     
  Misses        487      487

Files Changed	Coverage Δ
gwcs/coordinate_frames.py	`95.49% <100.00%> (+0.03%)`	⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Cadair

Thanks for decoupling this from #457!

Cadair · 2023-08-07T08:57:53Z

gwcs/tests/test_api.py

@@ -521,3 +521,22 @@ def test_coordinate_frame_api():

    pixel2 = wcs.invert(world)
    assert u.allclose(pixel2, 0*u.pix)
+
+
+def test_split_celestial_axes():


Should we also add a test for this for not-quantity?

Cadair · 2023-08-07T12:58:16Z

Curiously this seems to have not fixed the issue with my real production WCSes 🤔

Cadair · 2023-08-07T12:59:47Z

ok, more specifically, it works with _values but not without.

Cadair · 2023-08-07T13:01:48Z

In [33]: import astropy.modeling.models as m
    ...: from astropy.coordinates import ICRS
    ...: import gwcs.coordinate_frames as cf
    ...: import astropy.units as u
    ...: from gwcs import WCS
    ...: 
    ...: # Setup a model where the pixel axes are (lat, wave, lon)
    ...: spatial = m.Multiply(10*u.arcsec/u.pix) & m.Multiply(15*u.arcsec/u.pix)  # pretend this is a spatial model
    ...: compound = m.Linear1D(intercept=0*u.nm, slope=10*u.nm/u.pix) & spatial
    ...: forward = m.Mapping((1, 2, 0)) | compound | m.Mapping((2, 0, 1))
    ...: 
    ...: # Setup the output frame
    ...: celestial_frame = cf.CelestialFrame(axes_order=(0, 2), unit=(u.arcsec, u.arcsec), reference_frame=ICRS())
    ...: spectral_frame = cf.SpectralFrame(axes_order=(1,), unit=u.nm)
    ...: output_frame = cf.CompositeFrame([spectral_frame, celestial_frame])
    ...: 
    ...: input_frame = cf.CoordinateFrame(3, ["PIXEL"]*3, axes_order=list(range(3)), unit=[u.pix]*3)
    ...: 
    ...: wcs = WCS(forward, input_frame, output_frame)
    ...: input_pixel = [0*u.pix]*3
    ...: output_world = wcs.pixel_to_world_values(*input_pixel)
    ...: output_pixel = wcs.world_to_pixel_values(*output_world)

In [34]: output_world_objects = wcs.pixel_to_world(*input_pixel)

In [35]: wcs.world_to_pixel(*output_world_objects)
---------------------------------------------------------------------------
UnitsError                                Traceback (most recent call last)
Cell In[35], line 1
----> 1 wcs.world_to_pixel(*output_world_objects)

File ~/Git/gwcs/gwcs/api.py:315, in GWCSAPIMixin.world_to_pixel(self, *world_objects)
    311 def world_to_pixel(self, *world_objects):
    312     """
    313     Convert world coordinates to pixel values.
    314     """
--> 315     result = self.invert(*world_objects, with_units=True)
    317     if self.input_frame.naxes > 1:
    318         first_res = result[0]

File ~/Git/gwcs/gwcs/wcs.py:488, in WCS.invert(self, *args, **kwargs)
    485 try:
    486     # remove iterative inverse-specific keyword arguments:
    487     akwargs = {k: v for k, v in kwargs.items() if k not in _ITER_INV_KWARGS}
--> 488     result = self.backward_transform(*args, **akwargs)
    489 except (NotImplementedError, KeyError):
    490     result = self.numerical_inverse(*args, **kwargs, with_units=with_units)

File ~/.virtualenvs/dkist/lib/python3.11/site-packages/astropy/modeling/core.py:1125, in Model.__call__(self, *args, **kwargs)
   1120 # prepare for model evaluation (overridden in CompoundModel)
   1121 evaluate, inputs, broadcasted_shapes, kwargs = self._pre_evaluate(
   1122     *args, **kwargs
   1123 )
-> 1125 outputs = self._generic_evaluate(evaluate, inputs, fill_value, with_bbox)
   1127 # post-process evaluation results (overridden in CompoundModel)
   1128 return self._post_evaluate(
   1129     inputs, outputs, broadcasted_shapes, with_bbox, **kwargs
   1130 )

File ~/.virtualenvs/dkist/lib/python3.11/site-packages/astropy/modeling/core.py:1087, in Model._generic_evaluate(self, evaluate, _inputs, fill_value, with_bbox)
   1085     outputs = bbox.evaluate(evaluate, _inputs, fill_value)
   1086 else:
-> 1087     outputs = evaluate(_inputs)
   1088 return outputs

File ~/.virtualenvs/dkist/lib/python3.11/site-packages/astropy/modeling/core.py:3308, in CompoundModel._pre_evaluate.<locals>.evaluate(_inputs)
   3307 def evaluate(_inputs):
-> 3308     return self._evaluate(*_inputs, **kwargs)

File ~/.virtualenvs/dkist/lib/python3.11/site-packages/astropy/modeling/core.py:3351, in CompoundModel._evaluate(self, *args, **kw)
   3349 elif op == "|":
   3350     if isinstance(leftval, tuple):
-> 3351         return self.right(*leftval, **kw)
   3352     else:
   3353         return self.right(leftval, **kw)

File ~/.virtualenvs/dkist/lib/python3.11/site-packages/astropy/modeling/core.py:1125, in Model.__call__(self, *args, **kwargs)
   1120 # prepare for model evaluation (overridden in CompoundModel)
   1121 evaluate, inputs, broadcasted_shapes, kwargs = self._pre_evaluate(
   1122     *args, **kwargs
   1123 )
-> 1125 outputs = self._generic_evaluate(evaluate, inputs, fill_value, with_bbox)
   1127 # post-process evaluation results (overridden in CompoundModel)
   1128 return self._post_evaluate(
   1129     inputs, outputs, broadcasted_shapes, with_bbox, **kwargs
   1130 )

File ~/.virtualenvs/dkist/lib/python3.11/site-packages/astropy/modeling/core.py:1087, in Model._generic_evaluate(self, evaluate, _inputs, fill_value, with_bbox)
   1085     outputs = bbox.evaluate(evaluate, _inputs, fill_value)
   1086 else:
-> 1087     outputs = evaluate(_inputs)
   1088 return outputs

File ~/.virtualenvs/dkist/lib/python3.11/site-packages/astropy/modeling/core.py:3308, in CompoundModel._pre_evaluate.<locals>.evaluate(_inputs)
   3307 def evaluate(_inputs):
-> 3308     return self._evaluate(*_inputs, **kwargs)

File ~/.virtualenvs/dkist/lib/python3.11/site-packages/astropy/modeling/core.py:3336, in CompoundModel._evaluate(self, *args, **kw)
   3334 if op != "fix_inputs":
   3335     if op != "&":
-> 3336         leftval = self.left(*args, **kw)
   3337         if op != "|":
   3338             rightval = self.right(*args, **kw)

File ~/.virtualenvs/dkist/lib/python3.11/site-packages/astropy/modeling/core.py:1125, in Model.__call__(self, *args, **kwargs)
   1120 # prepare for model evaluation (overridden in CompoundModel)
   1121 evaluate, inputs, broadcasted_shapes, kwargs = self._pre_evaluate(
   1122     *args, **kwargs
   1123 )
-> 1125 outputs = self._generic_evaluate(evaluate, inputs, fill_value, with_bbox)
   1127 # post-process evaluation results (overridden in CompoundModel)
   1128 return self._post_evaluate(
   1129     inputs, outputs, broadcasted_shapes, with_bbox, **kwargs
   1130 )

File ~/.virtualenvs/dkist/lib/python3.11/site-packages/astropy/modeling/core.py:1087, in Model._generic_evaluate(self, evaluate, _inputs, fill_value, with_bbox)
   1085     outputs = bbox.evaluate(evaluate, _inputs, fill_value)
   1086 else:
-> 1087     outputs = evaluate(_inputs)
   1088 return outputs

File ~/.virtualenvs/dkist/lib/python3.11/site-packages/astropy/modeling/core.py:3308, in CompoundModel._pre_evaluate.<locals>.evaluate(_inputs)
   3307 def evaluate(_inputs):
-> 3308     return self._evaluate(*_inputs, **kwargs)

File ~/.virtualenvs/dkist/lib/python3.11/site-packages/astropy/modeling/core.py:3343, in CompoundModel._evaluate(self, *args, **kw)
   3340         rightval = None
   3342 else:
-> 3343     leftval = self.left(*(args[: self.left.n_inputs]), **kw)
   3344     rightval = self.right(*(args[self.left.n_inputs :]), **kw)
   3346 if op != "|":

File ~/.virtualenvs/dkist/lib/python3.11/site-packages/astropy/modeling/core.py:418, in __call__(self, model_set_axis, with_bounding_box, fill_value, equivalencies, inputs_map, *inputs, **new_inputs)
    409 args = ("self",)
    410 kwargs = {
    411     "model_set_axis": None,
    412     "with_bounding_box": False,
   (...)
    415     "inputs_map": None,
    416 }
--> 418 new_call = make_function_with_signature(
    419     __call__, args, kwargs, varargs="inputs", varkwargs="new_inputs"
    420 )
    422 # The following makes it look like __call__
    423 # was defined in the class
    424 update_wrapper(new_call, cls)

File ~/.virtualenvs/dkist/lib/python3.11/site-packages/astropy/modeling/core.py:395, in _ModelMeta._handle_special_methods.<locals>.__call__(self, *inputs, **kwargs)
    393 def __call__(self, *inputs, **kwargs):
    394     """Evaluate this model on the supplied inputs."""
--> 395     return super(cls, self).__call__(*inputs, **kwargs)

File ~/.virtualenvs/dkist/lib/python3.11/site-packages/astropy/modeling/core.py:1121, in Model.__call__(self, *args, **kwargs)
   1118 fill_value = kwargs.pop("fill_value", np.nan)
   1120 # prepare for model evaluation (overridden in CompoundModel)
-> 1121 evaluate, inputs, broadcasted_shapes, kwargs = self._pre_evaluate(
   1122     *args, **kwargs
   1123 )
   1125 outputs = self._generic_evaluate(evaluate, inputs, fill_value, with_bbox)
   1127 # post-process evaluation results (overridden in CompoundModel)

File ~/.virtualenvs/dkist/lib/python3.11/site-packages/astropy/modeling/core.py:969, in Model._pre_evaluate(self, *args, **kwargs)
    965 """
    966 Model specific input setup that needs to occur prior to model evaluation.
    967 """
    968 # Broadcast inputs into common size
--> 969 inputs, broadcasted_shapes = self.prepare_inputs(*args, **kwargs)
    971 # Setup actual model evaluation method
    972 parameters = self._param_sets(raw=True, units=True)

File ~/.virtualenvs/dkist/lib/python3.11/site-packages/astropy/modeling/core.py:2091, in Model.prepare_inputs(self, model_set_axis, equivalencies, *inputs, **kwargs)
   2087 self._validate_input_shapes(inputs, self.inputs, model_set_axis)
   2089 inputs_map = kwargs.get("inputs_map", None)
-> 2091 inputs = self._validate_input_units(inputs, equivalencies, inputs_map)
   2093 # The input formatting required for single models versus a multiple
   2094 # model set are different enough that they've been split into separate
   2095 # subroutines
   2096 if self._n_models == 1:

File ~/.virtualenvs/dkist/lib/python3.11/site-packages/astropy/modeling/core.py:2169, in Model._validate_input_units(self, inputs, equivalencies, inputs_map)
   2161             raise UnitsError(
   2162                 f"{name}: Units of input '{self.inputs[i]}', "
   2163                 f"{inputs[i].unit} ({inputs[i].unit.physical_type}),"
   (...)
   2166                 "input"
   2167             )
   2168         else:
-> 2169             raise UnitsError(
   2170                 f"{name}: Units of input '{self.inputs[i]}', "
   2171                 f"{inputs[i].unit} ({inputs[i].unit.physical_type}),"
   2172                 " could not be "
   2173                 "converted to required input"
   2174                 f" units of {input_unit} ({input_unit.physical_type})"
   2175             )
   2176 else:
   2177     # If we allow dimensionless input, we add the units to the
   2178     # input values without conversion, otherwise we raise an
   2179     # exception.
   2181     if (
   2182         not self.input_units_allow_dimensionless[input_name]
   2183         and input_unit is not dimensionless_unscaled
   2184         and input_unit is not None
   2185     ):

UnitsError: Linear1D: Units of input 'x', arcsec (angle), could not be converted to required input units of nm (length)

Cadair · 2023-10-12T21:19:13Z

Close this in favour of #457 ?

Allow splitting Celestial axes

9a797f8

nden requested a review from a team as a code owner August 6, 2023 20:31

Cadair reviewed Aug 7, 2023

View reviewed changes

Add a numerical only test

3350441

nden closed this Nov 17, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow splitting Celestial axes #467

Allow splitting Celestial axes #467

nden commented Aug 6, 2023 •

edited

codecov bot commented Aug 6, 2023 •

edited

Cadair left a comment

Cadair Aug 7, 2023

Cadair commented Aug 7, 2023

Cadair commented Aug 7, 2023

Cadair commented Aug 7, 2023

Cadair commented Oct 12, 2023

Allow splitting Celestial axes #467

Allow splitting Celestial axes #467

Conversation

nden commented Aug 6, 2023 • edited

codecov bot commented Aug 6, 2023 • edited

Codecov Report

Cadair left a comment

Choose a reason for hiding this comment

Cadair Aug 7, 2023

Choose a reason for hiding this comment

Cadair commented Aug 7, 2023

Cadair commented Aug 7, 2023

Cadair commented Aug 7, 2023

Cadair commented Oct 12, 2023

nden commented Aug 6, 2023 •

edited

codecov bot commented Aug 6, 2023 •

edited