drizzle icon indicating copy to clipboard operation
drizzle copied to clipboard

API re-design: simplify interface, remove I/O, make instrument agnostic

Open mcara opened this issue 1 year ago • 6 comments

~WARNING: This first commit is the first draft and it is definitely not ready for merging and unit tests have not been updated. Also, documentation is incomplete.~

This PR quite radically changes the API of this package with intention to make this package independent of how data are stored (FITS files, asdf, etc.) and the instrument used (JWST, HST, etc.) and also to reduce the number of functions and duplication and the number of conflicting parameters (or parameters that are changing meaning in different functions).

If some items on the TODO list (in the code) are implemented such as support for weight map of the input image to be None, then these changes would result in performance increase for EXPTIME, EXPSQ weighting schemes (as opposite to IVM). I would argue that we go even further with this in not having two parameters for input weights: weight_map and wht_scale. I think these should combine into a single weight which can be either an array or a scalar. IMO, having two parameters is an unnecessary complication.

BUG FIX: exposure time was undefined when in_units were not cps. That is, drizzle was producing random outputs when input image units were, let's say 'counts'.

BUG FIX: tdriz signature was out of date.

It is also worth mentioning this issue that probably should be handled in this PR too: https://github.com/spacetelescope/drizzlepac/issues/1722

mcara avatar Jan 19 '24 18:01 mcara

Codecov Report

Attention: Patch coverage is 98.61751% with 3 lines in your changes missing coverage. Please review.

Project coverage is 97.71%. Comparing base (0104065) to head (9fece19). Report is 14 commits behind head on main.

Files with missing lines Patch % Lines
drizzle/util.py 0.00% 2 Missing :warning:
drizzle/utils.py 98.14% 1 Missing :warning:
Additional details and impacted files
@@             Coverage Diff             @@
##             main     #134       +/-   ##
===========================================
+ Coverage   75.29%   97.71%   +22.42%     
===========================================
  Files           7        3        -4     
  Lines         344      219      -125     
===========================================
- Hits          259      214       -45     
+ Misses         85        5       -80     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Jan 19 '24 18:01 codecov[bot]

Example usage:

# wcs1 - WCS of the input image usually with distortions (to be resampled)
# wcs2 - WCS of the output image without distortions

import numpy as np

# simulate some data and a pixel map:
data = np.ones((240, 570))
y, x = np.indices((240, 570), dtype=np.float64)
pixmap = np.dstack([x, y])
# or call:
# from drizzle.utils import calc_pixmap
# pixmap = calc_pixmap(wcs1, wcs2)

# actual drizzle work
from drizzle.resample import Drizzle
d = Drizzle(out_shape=(240, 570))
d.add_image(data, exptime=1234, pixmap=pixmap)

# access outputs:
d.out_sci
d.out_ctx
d.out_wht

mcara avatar Jan 23 '24 17:01 mcara

Could there be an option to fold the pixmap computation inside the the call to add_image? This would be useful in cases where the pixmap is very large and would allow the method to do it for subarrays in cases of very large images (and complex GWCS models).

perrygreenfield avatar Jan 26 '24 20:01 perrygreenfield

Could there be an option to fold the pixmap computation inside the the call to add_image? This would be useful in cases where the pixmap is very large and would allow the method to do it for subarrays in cases of very large images (and complex GWCS models).

This is one possible approach to deal with large images. There also could be a different function (i.e., add_block()) that could deal with blocks of input images or caller could provide a callback, etc. I believe we should add parameters as we introduce the feature that needs those parameters. So, we could possibly add this later when we will support subarrays if this will be deemed to be the best approach.

The beauty of using pixmap is that it is completely independent on how WCS are computed and also keep in mind that pixmap size is the same as that of the input image and not of the output image and so maybe it is not that critical to be able to split input images into chunks.

mcara avatar Jan 29 '24 11:01 mcara

Section "The Drizzle Library" in the README.rst file has been removed because code examples are no longer valid/up-to-date with API changes. Updated examples should be provided either as a separate commit in this PR or a separate PR.

mcara avatar Mar 04 '24 17:03 mcara

I wonder whether https://github.com/spacetelescope/drizzlepac/pull/1767 should be ported to drizzle too (it is basically to get beautiful images with a fraction of pixels having potentially unusable values for science). Basically, instead of having one constant fillval, let's fill pixels with whatever we can get from input images even if those input pixels have 0 weight. It could be an option...

mcara avatar Mar 13 '24 19:03 mcara

Regression tests using this branch and jwst pipeline main are running here: https://plwishmaster.stsci.edu:8081/job/RT/job/JWST-Developers-Pull-Requests/1801/

These tests were failing because util.py was removed (the only call used by pipelines was to is_blank()). I restored it in 9fece19b74487f197784301a8b44c63fb4043089 and re-run regression test here: https://plwishmaster.stsci.edu:8081/job/RT/job/JWST-Developers-Pull-Requests/1802/

mcara avatar Oct 18 '24 20:10 mcara

Regression tests on romancal are running here: https://github.com/spacetelescope/RegressionTests/actions/runs/11410576103

mcara avatar Oct 18 '24 20:10 mcara