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

Resample may not be utilizing drizpars parameters #7125

Closed stscijgbot-jp closed 2 months ago

stscijgbot-jp commented 1 year ago

Issue JP-2580 was created on JIRA by Tyler Pauly:

While reviewing a PR against resample, I noticed that certain spec/drizpars parameters were only being taken from the reference file when the spec version was None - however, these parameters all have non-None default values. I tested this with the NIRCam drizpars reference file and a four-group association, which should set the pixel_scale to 0.8. Instead, the default spec value of 1.0 is used.

Some extra logging details from a test run:

2022-04-18 17:26:46,455 - stpipe.ResampleStep - INFO - Drizpars reference file: /Users/tpauly/crds_cache/references/jwst/nircam/jwst_nircam_drizpars_0001.fits
2022-04-18 17:26:46,490 - stpipe.ResampleStep - WARNING - num_groups: 4 ['jw01063101001_02101_1', 'jw01063101001_02101_2', 'jw01063101001_02101_3', 'jw01063101001_02101_4']
2022-04-18 17:26:46,490 - stpipe.ResampleStep - WARNING - default drizpars: {'pixfrac': 1.0, 'kernel': 'square', 'fillval': 'INDEF', 'pscale_ratio': 1.0}
2022-04-18 17:26:46,490 - stpipe.ResampleStep - WARNING - reffile_drizpars pixfrac: {'pixfrac': 0.8, 'kernel': 'square', 'fillval': 'INDEF', 'pscale_ratio': 1.0}
2022-04-18 17:26:46,490 - stpipe.ResampleStep - WARNING - kwargs pixfrac: 1.0
2022-04-18 17:26:46,718 - stpipe.ResampleStep - INFO - Blending metadata for four_image_resample_clear-f200w
stscijgbot-jp commented 1 year ago

Comment by Tyler Pauly on JIRA:

Alicia Canipe Misty Cracraft I noticed that the NIRCam and MIRI drizpars reference files set the pixfrac parameter to 0.8 when resampling four or more groups, yet I found that the pipeline overrides this value with its default value of 1.0 - do the NIRCam/MIRI teams also notice this behavior?

stscijgbot-jp commented 1 year ago

Comment by Misty Cracraft on JIRA:

I don't think we ever looked for it before, but I just checked one of the pipeline testing notebooks, and yes, the MIRI data also used a pixfrac of 1.0.

2022-03-24 13:53:46,395 - stpipe.Image3Pipeline.resample - INFO - Step resample parameters are: {'pre_hooks': [], 'post_hooks': [], 'output_file': None, 'output_dir': None, 'output_ext': '.fits', 'output_use_model': False, 'output_use_index': True, 'save_results': True, 'skip': False, 'suffix': 'i2d', 'search_output_file': True, 'input_dir': '', 'pixfrac': 1.0, 'kernel': 'square', 'fillval': 'INDEF', 'weight_type': 'ivm', 'output_shape': None, 'crpix': None, 'crval': None, 'rotation': None, 'pixel_scale_ratio': 1.0, 'pixel_scale': None, 'single': False, 'blendheaders': True, 'allowed_memory': None}

stscijgbot-jp commented 1 year ago

Comment by Jane Morrison on JIRA:

Tyler Pauly Howard Bushouse Mihai Cara 

There are a number of disconnects in how resample pulls in data from driz parameter reference file. I am not  expert on this topic so I want some input before I proceed.

I propose we  set all parameters in the spec to None if they are also in the driz_pars reference file. I think a clear description on JDOX will be needed so the user knows these values are by default read in from the reference files. 

        datatype: [ascii, 10]

The module get_drizpars in resample_step.py is just assuming the drizpars_table will have matching name. I would think we do not want to update the data model or the reference files - so that leaves updating the spec from 'weight_type' to 'wht_type'  unless someone has another suggestion

stscijgbot-jp commented 1 year ago

Comment by Jane Morrison on JIRA:

Ok I think I have  solution for the 'weight_type' issue. I just added self.wht_type and set it = self.weight_type. I changed  all self.weight_type references in the code to self.wht_type. I will put in a PR soon on this for comment.

stscijgbot-jp commented 1 year ago

Comment by Howard Bushouse on JIRA:

Warren Hack [X] Mihai Cara how are the param values handled on the HST side regarding the use the MDRIZTAB ref file (which is the equivalent of our drizpars ref file)? Are the params in astrodrizzle set to defaults of None rather than an actual default value, which then allows the values from MDRIZTAB to take precedence?

stscijgbot-jp commented 1 year ago

Comment by Howard Bushouse on JIRA:

6921 has made all of the code updates necessary to allow for use of params from the drizpars ref file, but has left the default values of the params in the step "spec" still set to their original values, which will cause the step to still ignore the drizpars ref file values. This is a temporary measure until the drizpars ref files get updated in CRDS to use more current, optimized values. When that happens, the param values in the step "spec" can be set to None. So this issue is still open.

stscijgbot-jp commented 1 year ago

Comment by Howard Bushouse on JIRA:

See also github issue #6984

stscijgbot-jp commented 5 months ago

Comment by David Law on JIRA:

Jane Morrison Howard Bushouse Is this ticket still relevant given https://jira.stsci.edu/browse/JP-2682 ?

stscijgbot-jp commented 5 months ago

Comment by Howard Bushouse on JIRA:

David Law Probably not. Once we have parameter reference files in place for resample/resample_spec those should override the undesired effects we're currently seeing with drizpars ref files, but it may be useful to keep this ticket open nonetheless, as a reminder that at some point we'll want to modify the resample/resample_spec step code to no longer even look for and try to use drizpars ref files (i.e. deprecate drizpars ref files all together).

stscijgbot-jp commented 5 months ago

Comment by Howard Bushouse on JIRA:

I'll set the status to on hold for now, while we're waiting for param ref files to be delivered and tested by the instrument teams.

stscijgbot-jp commented 2 months ago

Comment by Melanie Clarke on JIRA:

David Law - I think this ticket can be closed now. Drizpars files are no longer used and the instrument teams have delivered parameter reference files instead.