Respect the bounding_box in inverse transforms #498
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #498 +/- ##
==========================================
+ Coverage 87.42% 87.50% +0.08%
==========================================
Files 22 22
Lines 3874 3931 +57
==========================================
+ Hits 3387 3440 +53
- Misses 487 491 +4 ☔ View full report in Codecov by Sentry. |
|
@bmorris3 Do you mind testing this PR with your code? |
|
@Cadair Can you take a look at this PR and see if it's OK for sun related code? |
WilliamJamieson
force-pushed
the
inverse-bbox
branch
2 times, most recently
from
12692ac to
404d692
Compare
| not_numerical = True | ||
| world_arrays = high_level_objects_to_values(*world_arrays, low_level_wcs=self) | ||
| for axtyp in axes_types: | ||
| ind = np.asarray((np.asarray(self.output_frame.axes_type) == axtyp)) |
There was a problem hiding this comment.
I don't think outer np.asarray is needed.
| @@ -501,7 +489,57 @@ def invert(self, *args, **kwargs): | |||
| else: | |||
| return result | |||
|
|
|||
| def numerical_inverse(self, *args, tolerance=1e-5, maxiter=50, adaptive=True, | |||
| def outside_footprint(self, world_arrays): | |||
| return world_arrays | ||
|
|
||
|
|
||
| def out_of_bounds(self, pixel_arrays, fill_value=np.nan): |
There was a problem hiding this comment.
missing docstrings for a public function. Actually, maybe make these functions private?
| axis_range = footprint[:, idim] | ||
| else: | ||
| axis_range = footprint | ||
| range = [axis_range.min(), axis_range.max()] |
There was a problem hiding this comment.
This should be something like:
range = sorted([wrap_ra(axis_range.min(), wrapping_angle=180 or 360), wrap_ra(axis_range.max(), wrapping_angle=180 or 360)]or _dec (instead of _ra). To do this correctly, look into the transforms in gwcs pipeline and find CartesianToSpherical. Then look at CartesianToSpherical.wrap_lon_at to find the actual wrapping angle. However, this only for lon (RA). For lat I think the range is [-90, 90]. So a different treatment should be for that coordinate.
There was a problem hiding this comment.
Maybe something like this:
range = [axis_range.min(), axis_range.max()]
if abs(range[1] - range[0]) >= 350:
# most likely this coordinate is wrapped at 360
range = [range[1] - 360, range[0]]
elif abs(range[1] - range[0]) >= 170:
# most likely this coordinate is wrapped at 180
range = [range[1] - 180, range[0]]could detect phase jumps and unwrap one of the range's limit. This may need some tweaking.
There was a problem hiding this comment.
Also, range is a built-in function. I suggest changing the variable name.
Fixes #496
This fixes a bug in filtering out coordinates that are outside the footprint when evaluating the inverse transform. This affects also the
in_imagefunction. Previously the output of the backward transform was compared to the bounding_box and values outside it were replaced by NaNs. This PR adds an second step which is run before evaluating the backward transform and assigns output of NaN to inputs outside the footprint.