spacetelescope / jwst

Python library for science observations from the James Webb Space Telescope
https://jwst-pipeline.readthedocs.io/en/latest/
Other
562 stars 167 forks source link

Allow necessary image drizzling parameters to be passed into resample step #6301

Closed stscijgbot-jp closed 2 years ago

stscijgbot-jp commented 3 years ago

Issue JP-2261 was created on JIRA by Anton Koekemoer:

Necessary image drizzling parameters need to be able to be passed into the resample step, to be able to override the pipeline defaults (which would still be used if no such parameters are passed). This is a critical capability to enable users to carry out their science on the resampled images. For example, the WCS of the output frame needs to be able to be passed into the resample step, so that users can control the layout of resulting drizzled images (including, for example, the ability to drizzle images in different filters onto the same pixel grid, as well as the ability to specify the WCS in general). This capability is readily available in astrodrizzle and is widely used by the community, and offering a similar degree of flexibility in the jwst pipeline is critical to enable users to carry out their science. More details are provided in the comments section below.

stscijgbot-jp commented 1 year ago

Comment by Anton Koekemoer on JIRA:

For specifying the output WCS, the following parameters would need to be passed to the resample step, and, if present, these would be used to define the output WCS - some of these these are described here in terms of FITS keywords for clarity, but once they are passed into the resample code they can be translated into gwcs.WCS objects or other necessary quantities, as needed:

naxis1: x-dimension size of output drizzled image (units=pixels, type=int) naxis2: y-dimension size of output drizzled image (units=pixels, type=int) crpix1: x-coordinate of reference pixel in output drizzled image (units=pixels, type=float) crpix2: y-coordinate of reference pixel in output drizzled image (units=pixels, type=float) crval1: Right Ascension of reference pixel in output drizzled image (units=degrees, type=float) crval2: Declination of reference pixel in output drizzled image (units=degrees, type=float) rotation: Position angle of output drizzled image y-axis, relative to North, using the same rotation convention as the "final_rot" parameter in Astrodrizzle (units=degrees, type=float) pixel_scale: Pixel scale of output drizzled image, in both x and y (units=arcseconds, type=float)

The resample step would check whether the above parameters are provided (eg, via the parameter reference config file) – if they are provided then they would be used to construct the WCS of the output image (this would also include overriding the optional pixel_scale_ratio parameter) -- and if they are not provided then the resample step would construct the WCS of the output drizzled image in the same way as it currently does.

 

stscijgbot-jp commented 1 year ago

Comment by Howard Bushouse on JIRA:

FYI, for those keeping score via Jira filters, even though this ticket has been set to a status of "In Progress" (not sure why), no implementation work has actually started on it yet within SCSB. It's in the backlog, to be worked sometime in the next couple sprints.

stscijgbot-jp commented 1 year ago

Comment by Mihai Cara on JIRA:

PR is here: #6364

stscijgbot-jp commented 1 year ago

Comment by Anton Koekemoer on JIRA:

Thanks Mihai Cara  Howard Bushouse for the ongoing work on this. Was there meant to be an iteration with us about how you are implementing specifying the new parameters, so that we would have a chance to review and comment?

 

Currently I see the following seems to be implemented, for "crval", and this seems fine:

``--crval (tuple of float, default=None) Right ascension and declination of the reference pixel

So, specifically, this is perfectly fine, ie specifying Right Ascension first, followed by Declination.

 

However, I then see that for "shape" and "crpix" you have implemented them with the order reversed (y first, followed by x). From the perspective of users, I am certain that this will cause much confusion and many problems, and is also inconsistent with the order of RA,Dec given for crval.

 

Would it be possible please for you to implement "shape" and "crpix"so that they follow the same order as RA, Dec, ie with the x dimension first, followed by the y dimension? NOTE: I am aware that internally to python, the order of x,y does become swapped to y,x – however since these parameters are user-facing, we request please that their order maintains consistency with that of RA,Dec, ie x first, then y, and then the code should take care of swapping them internally.

stscijgbot-jp commented 1 year ago

Comment by Mihai Cara on JIRA:

in #6417 I switched order in output_shape to be traditional nx, ny. CRPIX was already in the x, y order.

stscijgbot-jp commented 1 year ago

Comment by Anton Koekemoer on JIRA:

thanks very much Mihai Cara  this now looks good, and thanks also for updating the documentation

https://jwst-pipeline.readthedocs.io/en/latest/jwst/resample/arguments.html

To summarize (just to capture it here in this ticket), these new parameters for the resample step are specified as follows (in addition to the other already existing parameters which are not repeated here):

 

--pixel_scale (float, default=None) Absolute pixel scale in arcsec. When provided, overrides pixel_scale_ratio.

--rotation (float, default=None) Position angle of output image’s Y-axis relative to North. A value of 0.0 would orient the final output image to be North up. The default of None specifies that the images will not be rotated, but will instead be resampled in the default orientation for the camera with the x and y axes of the resampled image corresponding approximately to the detector axes. Ignored when transform is provided.

--shape (tuple of int, default=None) Shape of the image (data array) using “standard” nx first and ny second. This value will be assigned to pixel_shape and array_shape properties of the returned WCS object. When supplied from command line, it should be a comma-separated list of integers nx, ny.

--crpix (tuple of float, default=None) Position of the reference pixel in the image array in the x, y order. If crpix is not specified, it will be set to the center of the bounding box of the returned WCS object. When supplied from command line, it should be a comma-separated list of floats.

--crval (tuple of float, default=None) Right Ascension and Declination of the reference pixel. Automatically computed if not provided. When supplied from command line, it should be a comma-separated list of floats.

stscijgbot-jp commented 1 year ago

Comment by Howard Bushouse on JIRA:

Who within the INS instrument teams is willing and able to test this implementation? Once a suitable volunteer has been identified, I'll reassign the ticket.

stscijgbot-jp commented 1 year ago

Comment by Swara Ravindranath on JIRA:

Howard Bushouse I will test this for NIRISS imaging.

stscijgbot-jp commented 1 year ago

Comment by Misty Cracraft on JIRA:

I did a very quick test with my resample notebook. I included values of rotation and output_shape as parameters when I ran calwebb_image3 and the output image was cut down to the size I indicated and rotated as specified. There can be more in-depth testing of all parameters, but this shows that the parameters that were opened to the users can be specified and propagate correctly into the pipeline.

stscijgbot-jp commented 1 year ago

Comment by Anton Koekemoer on JIRA:

thanks Misty Cracraft  and also Swara Ravindranath , this sounds great! I've tested it successfully also with NIRCam, where I ran it on sets of simulated SW and LW images in several different filters, specifying a common set of WCS values for all of them (customizing all the parameters namely pixel_scale, rotation, shape, crpix, crval) and they indeed all came out on matching pixel grids as expected.

stscijgbot-jp commented 1 year ago

Comment by Misty Cracraft on JIRA:

Since tests by Anton and Misty both indicated the resample parameters are working as expected, can we close this ticket? Anton Koekemoer do you agree that this issue can be closed?

stscijgbot-jp commented 1 year ago

Comment by Anton Koekemoer on JIRA:

From my side indeed this seems to work for NIRCam data, it might be good to just hear from Swara Ravindranath as well since she mentioned she would test it for NIRISS imaging?

stscijgbot-jp commented 1 year ago

Comment by Howard Bushouse on JIRA:

Can't imagine why the same code would work for one instrument but not another, but if Swara Ravindranath has done a test it can't hurt to hear about it.