spacetelescope / jwst

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

Include all parameters used for resample step in the config file #4764

Closed stscijgbot closed 3 years ago

stscijgbot commented 4 years ago

Issue JP-1400 was created by Swara Ravindranath:

The config file for resample step only includes some parameters and not all of them that are used in the drizzle step. For instance, the drizzle scale parameter which is the size of the output pixels relative to the input pixels is not listed in the config file. The parameter is default to 1.0, but users would want to drizzle to a finer pixel scale for dithered images to reconstruct the fully-sampled PSF.  This ticket is to include all the parameters used in the resample step to the config file so they are visible the users. The user should be able to change the parameters (eg; change scale= 1.0 to scale = 0.5), and run the pipeline steps on the command line using the updated values.

stscijgbot commented 4 years ago

Comment by Jonathan Eisenhamer: Question: How would this interact/overlap with the drizpars reference file? Should the reference file be modified instead? Or just the step parameters added? Or both?

stscijgbot commented 4 years ago

Comment by Swara Ravindranath: The drizzle parameters should also be exposed for the resample_spec step used in Calwebb_spec2 and Calwebb_spec3. I just added that to the "software affected" labels.

Just want to note that this should also be updated in the pipeline documentation.

[https://jwst-pipeline.readthedocs.io/en/latest/jwst/pipeline/calwebb_spec2.html]

According to the table on this documentation page, resample_spec is only applied for NIRSpec. This must be corrected to include NIRISS and NIRCam WFSS.

stscijgbot commented 4 years ago

Comment by James Davies: WFSS spectral data is not currently resampled in the spec2 or spec3 pipelines. So the documents are up-to-date.

As far as I know, we don't know of a good way to resample it currently. I.e. the algorithm is unknown. What does an output "rectified" slitless grism image look like? If someone who knows about grisms and how the WCSs of these work would like to discuss this, it would be good to do so, because this is work that should be done before launch I suspect. And it is not trivial work.

As for scale values other than 1, there's lots of down-stream ramifications of this. For instance the aperture correction reference files have aperture radii in pixels. If resampled output has a different pixel scale than the detector pixel scale, then, many (most) of the downstream steps need to be updated as well. In the case of spec2, that means backing out and rerunning a good part of the spec2 pipeline. So implementing this is not as simple as just exposing the parameter via the top-level interface.

Finally, the drizpars reffile should probably be retired after we have pars-resamplestep reference files available.

stscijgbot commented 4 years ago

Comment by James Davies: WFSS spectral data is not currently resampled in the spec2 or spec3 pipelines. So the documents are up-to-date.

As far as I know, we don't know of a good way to resample it currently. I.e. the algorithm is unknown. What does an output "rectified" slitless grism image look like? If someone who knows about grisms and how the WCSs of these work would like to discuss this, it would be good to do so, because this is work that should be done before launch if resampling is desired. And it is not trivial work.

As for scale values other than 1, there's lots of down-stream ramifications of this. For instance the aperture correction reference files have aperture radii in pixels. If resampled output has a different pixel scale than the detector pixel scale, then, many (most) of the downstream steps need to be updated as well. In the case of spec2, that means backing out and rerunning a good part of the spec2 pipeline. So implementing this is not as simple as just exposing the parameter via the top-level interface.

Finally, the drizpars reffile should probably be retired after we have pars-resamplestep reference files available.

stscijgbot commented 4 years ago

Comment by Kevin Volk: We are going to need the flexibility of using different drizzle parameters.  For example, currently we cannot do sub-pixel sampling as that parameter is not exposed, which defeats the purpose of designing the dithers to have half-pixel offsets to sub-sample the PSF.  This is quite important for science.  We might be able to get by in commissioning without this feature, but even then it would much better to have this capability in place.

stscijgbot commented 4 years ago

Comment by Howard Bushouse: Implementation wise, what's really needed here is to add the available drizzle params to the "spec" definition block in the resample_step.py module, which currently only consists of: {code:java} spec = """ pixfrac = float(default=1.0) kernel = string(default='square') fillval = string(default='INDEF') weight_type = option('exptime', default='exptime') single = boolean(default=False) blendheaders = boolean(default=True) """ {code} Adding params to the resample.cfg file without defining them in the "spec" block will not accomplish anything, because they still won't be recognized by the step as valid params. And once they are added to the "spec" definitions, there's no need to add them with default values in the .cfg file, because the defaults are already specified in the definition. The only time they need to be listed in a cfg file is when a user desires to use non-default values. Users can obtain a listing of the available params at any time for any step or pipeline via: {code:java} strun resample.cfg -h{code} or {code:java} strun jwst.resample.ResampleStep -h{code} both of which return {code:java} usage: strun [-h] [--logcfg LOGCFG] [--verbose] [--debug] [--save-parameters SAVE_PARAMETERS] [--disable-crds-steppars] [--pre_hooks] [--post_hooks]              [--output_file] [--output_dir] [--output_ext] [--output_use_model] [--output_use_index] [--save_results] [--skip] [--suffix]              [--search_output_file] [--input_dir] [--pixfrac] [--kernel] [--fillval] [--weight_type] [--single] [--blendheaders]              [--override_drizpars]              cfg_file_or_class [args [args ...]]

Resample input data onto a regular grid using the drizzle algorithm. Parameters ----------- input : DataModel or Association Single filename for either a single image or an association table.

positional arguments:   cfg_file_or_class     The configuration file or Python class to run   args                  arguments to pass to step

optional arguments:   -h, --help            show this help message and exit   --logcfg LOGCFG       The logging configuration file to load   --verbose, -v         Turn on all logging messages   --debug               When an exception occurs, invoke the Python debugger, pdb   --save-parameters SAVE_PARAMETERS                         Save step parameters to specified file.   --disable-crds-steppars                         Disable retrieval of step parameter references files from CRDS   --pre_hooks   --post_hooks   --output_file         File to save output to.   --output_dir          Directory path for output files   --output_ext          Default type of output   --output_use_model    When saving use DataModel.meta.filename   --output_use_index    Append index.   --save_results        Force save results   --skip                Skip this step   --suffix              Default suffix for output files   --search_output_file                         Use outputfile define in parent step   --input_dir           Input directory   --pixfrac   --kernel   --fillval   --weight_type   --single   --blendheaders   --override_drizpars   Override the drizpars reference file {code} (sort of the functional equivalent of the IRAF "lpar").

stscijgbot commented 4 years ago

Comment by Anton Koekemoer: This was discussed in [JWST Cal WG meeting 2020-08-25|https://outerspace.stsci.edu/display/JWSTCC/2020-08-25+Meeting+notes] and is accepted for the pipeline version expected for offline use by launch, ie post-B7.6, remaining as critical for science since this capability will likely be needed by early science users including the ERS teams and others, so the "calwg" label has been set, and this work will be further tracked by [~acanipe] in the DMS WG.

stscijgbot commented 3 years ago

Comment by James Davies: So implementing scaling in image resampling is straightforward. We've done it before in {{drizzlepac}}, and it is straightforward here for JWST.

For spectral data, what do we want to scale? Just the spatial (cross-dispersion) dimension, i.e. where the nodding might be sub-pixel, and we want to recover some of the diffraction-limited PSF?

What about the spectral (dispersion) direction? My thought is that it makes no sense to scale in the dispersion direction, as the resolution there is limited by the width of the slit. And the slit width better have been matched well to the native pixel scale by the disperser, right? I.e. there's nothing to be gained by scaling in the spectral direction. Or is there?

And to be clear, we're talking about NIRSpec FS, NIRSpec MOS and MIRI LRS FS here.

Would like to hear some thoughts.

stscijgbot commented 3 years ago

Comment by Misty Cracraft: [~muzerol] [~dlaw]  Do you have any thoughts on James' latest questions? If someone else would be able to answer, please tag them as well.

stscijgbot commented 3 years ago

Comment by David Law: Hm, it's sometimes the case that we can gain from scaling/oversampling in the spectral dimension as well.  The MRS is spectrally undersampled at some wavelengths, and dithering helps to fill in spectral sampling of the LSF as well as spatial sampling of the PSF.  MRS is not affected by this current ticket though, and is dealt with through cube_build rather than the resample step.  I don't think this is an issue for the LRS (though [~skendrew] could correct me), and have no idea about the other modes.

Regarding the impact on aperture corrections, I'd suggest that having radii given in units of arcsec obviates any kind of scale problems.

stscijgbot commented 3 years ago

Comment by James Muzerolle: Yes, NIRSpec MOS and FS should have the option to oversample in the spectral dimension.  We are undersampled both spatially and spectrally over much of our operable wavelength range, and sub-pixel dithering in the dispersion direction should help improve the LSF sampling (in fact, FS mode has a sub-pixel dither pattern specifically for this purpose).

jdavies-st commented 3 years ago

Thanks for the input David and James.

James, For NIRSpec MOS and FS, the recovery of LSF resolution via subpixel dithering would only be possible for point sources in a wide-ish slit, right? For extended sources, you're just imaging the full-width of the slit, so any subpixel resampling won't give any improvement, right? Or am I not thinking about this correctly?

Regardless, the current implementation (once merged) scales in the both the spatial and spectral directions. And that scaling is the same for spatial and spectral directions.

stscijgbot commented 3 years ago

Comment by James Muzerolle: [~jdavies] Yes, that's right, spectral sub-pixel dithering is only relevant for point sources.

stscijgbot commented 3 years ago

Comment by James Davies: A couple extra details that are required for this issue to be resolved:

stscijgbot commented 3 years ago

Comment by Howard Bushouse: Fixed by https://github.com/spacetelescope/jwst/pull/5389

stscijgbot commented 3 years ago

Comment by Swara Ravindranath: Tested the calwebb_image3 resample step in development version 0.18.2.dev14+gacf778f6. There is now the option to set the pixel_scale_ratio to a user-supplied value. Tested the resample step using a value of pixel_scale_ratio = 0.5.

The output image looks correct and is on the half-pixel drizzled scale.

For reference, the versions used are;

astropy: 4.2

asdf: 2.7.1

jwst: 0.18.2.dev14+gacf778f6   Yet to check the photometry and confirm that the aperture size definitions for encircled energy and reported fluxes are correct in the resampled image with 0.5 pixel scale.  

 

stscijgbot commented 3 years ago

Comment by Howard Bushouse: Assigned back to [~swara] to confirm whether all testing has been completed. If so, this can be closed.

stscijgbot commented 3 years ago

Comment by Anton Koekemoer: [~bushouse] and [~jdavies]  – a question has come up recently from the community about specifically which parameters are now 'exposed', and how to access them, in order for the resample step to achieve similar flexibility to drizzle, especially when running offline.

In particular, when running offline, how can the user tell the resample step to produce a drizzled image with a particular WCS, in terms of specifying the output tangent point location (CRPIX1,2 and CRVAL1,2), and output size (NAXIS1,2)?

stscijgbot commented 3 years ago

Comment by James Davies: One cannot do this currently. It is not a use case for automated pipeline processing, which has been the focus of development up until now.

But clearly we will want this for offline processing. It should be prioritized at some point.

stscijgbot commented 3 years ago

Comment by Anton Koekemoer: [~jdavies]  thanks for replying - and, to clarify, the idea of enabling the user to adjust parameters for the resample step (ie this ticket) is indeed not for automated pipeline processing (where, for example, the pixel scale is not being adjuted) but for offline use.

So offline processing is the use case for this ticket (as in the description).

 

stscijgbot commented 3 years ago

Comment by Anton Koekemoer: [~bushouse] and [~jdavies]  – a question has come up recently from the community about specifically which parameters are now 'exposed', and how to access them, in order for the resample step to achieve similar flexibility to drizzle, especially when running offline.

In particular, when running offline, how can the user tell the resample step to produce a drizzled image with a particular WCS, in terms of specifying the output tangent point location (CRPIX1,2 and CRVAL1,2), rotation w.r.t. north, and output size (NAXIS1,2)?

stscijgbot commented 3 years ago

Comment by Anton Koekemoer: To add more specifics, "self.output_wcs" in resample.py is the relevant WCS item to expose, ie for you to provide a way for the user to pass this to resample.py (or, alternatively, for the user to pass the parameters of CRVAL1,2 / CRPIX1,2 / NAXIS1,2 / Rotation w.r.t. north) and only have resample.py calculate this (using resample_utils.make_output_wcs) if it's not provided.

stscijgbot commented 3 years ago

Comment by Anton Koekemoer: Exposing this capability for the resample step seems to be the only way for users to generate drizzled mosaics that are all aligned to the same pixel grid even when the exposures are at different pointings, including cases where they are different filters or instruments (a standard capability in astrodrizzle); since this is not produced by the operational pipeline, this ticket therefore has a calwg priority of 'critical' for users to be able to carry our their science

stscijgbot commented 3 years ago

Comment by James Davies: There should be a separate ticket for user-supplied WCS, as that's extra functionality that does not currently exist, i.e. there is no

Also the WCS used in {{jwst}} are {{gwcs.WCS}} objects. There would need to be a little extra infrastructure work to be able to use FITS WCS (CD matrix, CRVAL, CRPIX) as input, and that is above and beyond just exposing it via the top-level API.

stscijgbot commented 3 years ago

Comment by Anton Koekemoer: [~jdavies] I've now set up a ticket for this, linking it to this one since the issues remain related: https://jira.stsci.edu/browse/JP-2261

stscijgbot commented 2 years ago

Comment by James Davies [X]: WFSS spectral data is not currently resampled in the spec2 or spec3 pipelines. So the documents are up-to-date.

As far as I know, we don't know of a good way to resample it currently. I.e. the algorithm is unknown. What does an output "rectified" slitless grism image look like? If someone who knows about grisms and how the WCSs of these work would like to discuss this, it would be good to do so, because this is work that should be done before launch if resampling is desired. And it is not trivial work.

As for scale values other than 1, there's lots of down-stream ramifications of this. For instance the aperture correction reference files have aperture radii in pixels. If resampled output has a different pixel scale than the detector pixel scale, then, many (most) of the downstream steps need to be updated as well. In the case of spec2, that means backing out and rerunning a good part of the spec2 pipeline. So implementing this is not as simple as just exposing the parameter via the top-level interface.

Finally, the drizpars reffile should probably be retired after we have pars-resamplestep reference files available.

stscijgbot commented 2 years ago

Comment by James Davies [X]: So implementing scaling in image resampling is straightforward. We've done it before in {{drizzlepac}}, and it is straightforward here for JWST.

For spectral data, what do we want to scale? Just the spatial (cross-dispersion) dimension, i.e. where the nodding might be sub-pixel, and we want to recover some of the diffraction-limited PSF?

What about the spectral (dispersion) direction? My thought is that it makes no sense to scale in the dispersion direction, as the resolution there is limited by the width of the slit. And the slit width better have been matched well to the native pixel scale by the disperser, right? I.e. there's nothing to be gained by scaling in the spectral direction. Or is there?

And to be clear, we're talking about NIRSpec FS, NIRSpec MOS and MIRI LRS FS here.

Would like to hear some thoughts.

stscijgbot commented 2 years ago

Comment by James Davies [X]: A couple extra details that are required for this issue to be resolved:

stscijgbot commented 2 years ago

Comment by James Davies [X]: One cannot do this currently. It is not a use case for automated pipeline processing, which has been the focus of development up until now.

But clearly we will want this for offline processing. It should be prioritized at some point.

stscijgbot commented 2 years ago

Comment by James Davies [X]: There should be a separate ticket for user-supplied WCS, as that's extra functionality that does not currently exist, i.e. there is no

Also the WCS used in {{jwst}} are {{gwcs.WCS}} objects. There would need to be a little extra infrastructure work to be able to use FITS WCS (CD matrix, CRVAL, CRPIX) as input, and that is above and beyond just exposing it via the top-level API.