AL-552: Tests and step code needed to change to accommodate for new parameter in `ramp_fit`. #6072

kmacdonald-stsci · 2021-05-25T16:17:49Z

Resolves AL-552

Description

To make ramp fitting code pipeline agnostic DQ flags are now passed as a parameter to ramp_fit. The new parameter to ramp_fit required changes to the tests and step code. All JWST ramp fit unit tests and regression tests pass with these changes.

Checklist

…ure.

jdavies-st · 2021-05-26T13:35:57Z

jwst/ramp_fitting/ramp_fit_step.py

+            ramp_fit_dq_flags = {
+                "DO_NOT_USE": dqflags.pixel["DO_NOT_USE"],
+                "JUMP_DET": dqflags.pixel["JUMP_DET"],
+                "SATURATED": dqflags.pixel["SATURATED"],
+                "NO_GAIN_VALUE": dqflags.pixel["NO_GAIN_VALUE"],
+                "UNRELIABLE_SLOPE": dqflags.pixel["UNRELIABLE_SLOPE"],
+            }


I think originally some of these DQ flags were from jwst.datamodels.dqflags.group dict.

jwst/jwst/ramp_fitting/ramp_fit.py

Lines 33 to 34 in f8d9122

DO_NOT_USE = dqflags.group['DO_NOT_USE']

JUMP_DET = dqflags.group['JUMP_DET']

And some from the pixel dict:

jwst/jwst/ramp_fitting/ramp_fit.py

Line 1547 in f8d9122

temp_dq[:, :][v_mask] = dqflags.pixel['UNRELIABLE_SLOPE']

Though it seems the group and pixel dq flags are identical. (above code snippets from release 1.1.0)

Is there a reason to redefine them here in the calling code instead of just shuffling the full dictionary on to ramp_fit and letting it use them as it used them before?

I changed the call to ramp_fit to simply pass dqflags.pixel, rather than create a separate dictionary to pass.

jdavies-st · 2021-05-26T13:37:34Z

jwst/ramp_fitting/tests/test_ramp_fit.py

+test_dq_flags = {
+    "DO_NOT_USE": dqflags.pixel["DO_NOT_USE"],
+    "JUMP_DET": dqflags.pixel["JUMP_DET"],
+    "SATURATED": dqflags.pixel["SATURATED"],
+    "NO_GAIN_VALUE": dqflags.pixel["NO_GAIN_VALUE"],
+    "UNRELIABLE_SLOPE": dqflags.pixel["UNRELIABLE_SLOPE"],
+}


Again, no need for this. Just use the existing dicts in jwst.datamodels.dqflags.pixel and jwst.datamodels.dqflags.group.

I changed the code to test_dq_flags = dqflags.pixel.

jdavies-st · 2021-05-26T13:37:57Z

jwst/ramp_fitting/tests/test_ramp_fit_cases.py

+test_dq_flags = {
+    "DO_NOT_USE": dqflags.pixel["DO_NOT_USE"],
+    "JUMP_DET": dqflags.pixel["JUMP_DET"],
+    "SATURATED": dqflags.pixel["SATURATED"],
+    "NO_GAIN_VALUE": dqflags.pixel["NO_GAIN_VALUE"],
+    "UNRELIABLE_SLOPE": dqflags.pixel["UNRELIABLE_SLOPE"],
+}


I changed the code to test_dq_flags = dqflags.pixel.

codecov · 2021-05-27T03:59:57Z

Codecov Report

Merging #6072 (3395dce) into master (8a8f148) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #6072   +/-   ##
=======================================
  Coverage   77.54%   77.55%           
=======================================
  Files         402      402           
  Lines       34419    34422    +3     
=======================================
+ Hits        26691    26696    +5     
+ Misses       7728     7726    -2

Flag	Coverage Δ
nightly	`77.56% <100.00%> (+<0.01%)`	⬆️
unit	`55.13% <100.00%> (+0.01%)`	⬆️

Impacted Files	Coverage Δ
jwst/ramp_fitting/ramp_fit_step.py	`97.80% <100.00%> (+5.75%)`	⬆️
jwst/regtest/regtestdata.py	`82.06% <0.00%> (-1.35%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8a8f148...3395dce. Read the comment docs.

jdavies-st

This looks good.

This PR assumes that dqflags.group dict is a subset of dqflags.pixel and that those mnemonics are interchangeable. Which they currently are. And that was the intention, ~~but that is not actually formalized.~~

So I think this is OK.

Approving, though merging will have to wait until all the changes in stcal are merged and released.

~~Will file a separate issue/PR for formalize dqflags.pixel being a superset of dqflags.group.~~

EDIT: It actually is formalized.

jwst/jwst/datamodels/dqflags.py

Lines 25 to 70 in 4bd9e5a

    
           # Pixel-specific flags 
        
           pixel = {'GOOD':             0,      # No bits set, all is good 
        
                    'DO_NOT_USE':       2**0,   # Bad pixel. Do not use. 
        
                    'SATURATED':        2**1,   # Pixel saturated during exposure 
        
                    'JUMP_DET':         2**2,   # Jump detected during exposure 
        
                    'DROPOUT':          2**3,   # Data lost in transmission 
        
                    'OUTLIER':          2**4,   # Flagged by outlier detection (was RESERVED_1) 
        
                    'PERSISTENCE':      2**5,   # High persistence (was RESERVED_2) 
        
                    'AD_FLOOR':         2**6,   # Below A/D floor (0 DN, was RESERVED_3) 
        
                    'RESERVED_4':       2**7,   # 
        
                    'UNRELIABLE_ERROR': 2**8,   # Uncertainty exceeds quoted error 
        
                    'NON_SCIENCE':      2**9,   # Pixel not on science portion of detector 
        
                    'DEAD':             2**10,  # Dead pixel 
        
                    'HOT':              2**11,  # Hot pixel 
        
                    'WARM':             2**12,  # Warm pixel 
        
                    'LOW_QE':           2**13,  # Low quantum efficiency 
        
                    'RC':               2**14,  # RC pixel 
        
                    'TELEGRAPH':        2**15,  # Telegraph pixel 
        
                    'NONLINEAR':        2**16,  # Pixel highly nonlinear 
        
                    'BAD_REF_PIXEL':    2**17,  # Reference pixel cannot be used 
        
                    'NO_FLAT_FIELD':    2**18,  # Flat field cannot be measured 
        
                    'NO_GAIN_VALUE':    2**19,  # Gain cannot be measured 
        
                    'NO_LIN_CORR':      2**20,  # Linearity correction not available 
        
                    'NO_SAT_CHECK':     2**21,  # Saturation check not available 
        
                    'UNRELIABLE_BIAS':  2**22,  # Bias variance large 
        
                    'UNRELIABLE_DARK':  2**23,  # Dark variance large 
        
                    'UNRELIABLE_SLOPE': 2**24,  # Slope variance large (i.e., noisy pixel) 
        
                    'UNRELIABLE_FLAT':  2**25,  # Flat variance large 
        
                    'OPEN':             2**26,  # Open pixel (counts move to adjacent pixels) 
        
                    'ADJ_OPEN':         2**27,  # Adjacent to open pixel 
        
                    'UNRELIABLE_RESET': 2**28,  # Sensitive to reset anomaly 
        
                    'MSA_FAILED_OPEN':  2**29,  # Pixel sees light from failed-open shutter 
        
                    'OTHER_BAD_PIXEL':  2**30,  # A catch-all flag 
        
                    'REFERENCE_PIXEL':  2**31,  # Pixel is a reference pixel 
        
                    } 
        
           # Group-specific flags. Once groups are combined, these flags 
        
           # are equivalent to the pixel-specific flags. 
        
           group = {'GOOD':       pixel['GOOD'], 
        
                    'DO_NOT_USE': pixel['DO_NOT_USE'], 
        
                    'SATURATED':  pixel['SATURATED'], 
        
                    'JUMP_DET':   pixel['JUMP_DET'], 
        
                    'DROPOUT':    pixel['DROPOUT'], 
        
                    'AD_FLOOR':   pixel['AD_FLOOR'], 
        
                    }

🤦

I clearly need more ☕ .

jdavies-st · 2021-05-27T20:17:16Z

FWIW, dqflags.group being a subset of dqflags.pixel was formalized in #4726

hbushouse

Needs a change log entry (in CHANGES.rst)

hbushouse · 2021-07-19T19:04:43Z

jwst/ramp_fitting/ramp_fit_step.py

@@ -162,10 +165,13 @@ def create_optional_results_model(opt_info):
        weights=weights,
        crmag=crmag)

+    opt_model.meta.filename = input_model.meta.filename
+    opt_model.update(input_model)  # ... and add all keys from input


I think that calling .update will include the copying of meta.filename from the input to the output model, so line 168 may not even be necessary. Also, do we really want to set the input and output file names to be the same? Shouldn't the opt_model get saved to a file name that at least has a different file type suffix? Or is that handled later?

That's done later in the Step code:

jwst/jwst/ramp_fitting/ramp_fit_step.py

Lines 240 to 243 in 8a8f148

# Save the OLS optional fit product, if it exists

if opt_info is not None:

opt_model = create_optional_results_model(opt_info)

self.save_model(opt_model, 'fitopt', output_file=self.opt_name)

jdavies-st · 2021-07-20T00:59:51Z

Unit and regression tests pass pointing to stcal 0.2.2

https://plwishmaster.stsci.edu:8081/job/RT/job/jdavies-dev/262/

Merging.

…arameter in `ramp_fit`. (spacetelescope#6072) * Add DQ flags parameter to due to the change in the function signature. * Updated optional results product to get keywords and meta data. * Temporarily point to stcal/main in setup.cfg * Fix bug in create_optional_results_model * Add entry to CHANGES.rst * Require stcal>=0.2.2 for ramp_fit dqflag api change Co-authored-by: James Davies <jdavies@stsci.edu>

Adding DQ flags parameter to due to the change in the function signat…

2999f13

…ure.

kmacdonald-stsci requested review from nden, dmggh, hbushouse and jdavies-st May 25, 2021 16:17

kmacdonald-stsci mentioned this pull request May 25, 2021

Al-522: Adding a DQ flags parameter to ramp_fit to make them pipeline indepedent spacetelescope/stcal#25

Merged

jdavies-st added the ramp_fitting label May 26, 2021

jdavies-st reviewed May 26, 2021

View reviewed changes

Changing DQ flag parameters based on code review.

4eb3508

jdavies-st added this to the Build 7.8 milestone May 27, 2021

jdavies-st approved these changes May 27, 2021

View reviewed changes

Updated optional results product to get keywords and meta data.

e37a9a4

nden modified the milestones: Build 7.8, Build 7.9 Jun 21, 2021

jdavies-st added 2 commits July 19, 2021 13:06

Temporarily point to stcal/main in setup.cfg

148b4bd

Fix bug in create_optional_results_model

9d04156

hbushouse reviewed Jul 19, 2021

View reviewed changes

jdavies-st added 3 commits July 19, 2021 16:32

Add entry to CHANGES.rst

e0181d3

Require stcal>=0.2.2 for ramp_fit dqflag api change

da9c879

Merge branch 'master' into al_522_dqflags

3395dce

jdavies-st merged commit 86d28aa into spacetelescope:master Jul 20, 2021

kmacdonald-stsci deleted the al_522_dqflags branch July 26, 2021 16:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

AL-552: Tests and step code needed to change to accommodate for new parameter in `ramp_fit`. #6072

AL-552: Tests and step code needed to change to accommodate for new parameter in `ramp_fit`. #6072

kmacdonald-stsci commented May 25, 2021 •

edited by jdavies-st

Loading

jdavies-st May 26, 2021

kmacdonald-stsci May 26, 2021

jdavies-st May 26, 2021

kmacdonald-stsci May 26, 2021

jdavies-st May 26, 2021

kmacdonald-stsci May 26, 2021

codecov bot commented May 27, 2021 •

edited

Loading

jdavies-st left a comment •

edited

Loading

jdavies-st commented May 27, 2021

hbushouse left a comment

hbushouse Jul 19, 2021

jdavies-st Jul 19, 2021

jdavies-st commented Jul 20, 2021

	DO_NOT_USE = dqflags.group['DO_NOT_USE']
	JUMP_DET = dqflags.group['JUMP_DET']

	# Pixel-specific flags
	pixel = {'GOOD': 0, # No bits set, all is good
	'DO_NOT_USE': 2**0, # Bad pixel. Do not use.
	'SATURATED': 2**1, # Pixel saturated during exposure
	'JUMP_DET': 2**2, # Jump detected during exposure
	'DROPOUT': 2**3, # Data lost in transmission
	'OUTLIER': 2**4, # Flagged by outlier detection (was RESERVED_1)
	'PERSISTENCE': 2**5, # High persistence (was RESERVED_2)
	'AD_FLOOR': 2**6, # Below A/D floor (0 DN, was RESERVED_3)
	'RESERVED_4': 2**7, #
	'UNRELIABLE_ERROR': 2**8, # Uncertainty exceeds quoted error
	'NON_SCIENCE': 2**9, # Pixel not on science portion of detector
	'DEAD': 2**10, # Dead pixel
	'HOT': 2**11, # Hot pixel
	'WARM': 2**12, # Warm pixel
	'LOW_QE': 2**13, # Low quantum efficiency
	'RC': 2**14, # RC pixel
	'TELEGRAPH': 2**15, # Telegraph pixel
	'NONLINEAR': 2**16, # Pixel highly nonlinear
	'BAD_REF_PIXEL': 2**17, # Reference pixel cannot be used
	'NO_FLAT_FIELD': 2**18, # Flat field cannot be measured
	'NO_GAIN_VALUE': 2**19, # Gain cannot be measured
	'NO_LIN_CORR': 2**20, # Linearity correction not available
	'NO_SAT_CHECK': 2**21, # Saturation check not available
	'UNRELIABLE_BIAS': 2**22, # Bias variance large
	'UNRELIABLE_DARK': 2**23, # Dark variance large
	'UNRELIABLE_SLOPE': 2**24, # Slope variance large (i.e., noisy pixel)
	'UNRELIABLE_FLAT': 2**25, # Flat variance large
	'OPEN': 2**26, # Open pixel (counts move to adjacent pixels)
	'ADJ_OPEN': 2**27, # Adjacent to open pixel
	'UNRELIABLE_RESET': 2**28, # Sensitive to reset anomaly
	'MSA_FAILED_OPEN': 2**29, # Pixel sees light from failed-open shutter
	'OTHER_BAD_PIXEL': 2**30, # A catch-all flag
	'REFERENCE_PIXEL': 2**31, # Pixel is a reference pixel
	}


	# Group-specific flags. Once groups are combined, these flags
	# are equivalent to the pixel-specific flags.
	group = {'GOOD': pixel['GOOD'],
	'DO_NOT_USE': pixel['DO_NOT_USE'],
	'SATURATED': pixel['SATURATED'],
	'JUMP_DET': pixel['JUMP_DET'],
	'DROPOUT': pixel['DROPOUT'],
	'AD_FLOOR': pixel['AD_FLOOR'],
	}

	# Save the OLS optional fit product, if it exists
	if opt_info is not None:
	opt_model = create_optional_results_model(opt_info)
	self.save_model(opt_model, 'fitopt', output_file=self.opt_name)

AL-552: Tests and step code needed to change to accommodate for new parameter in ramp_fit. #6072

AL-552: Tests and step code needed to change to accommodate for new parameter in ramp_fit. #6072

Conversation

kmacdonald-stsci commented May 25, 2021 • edited by jdavies-st Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented May 27, 2021 • edited Loading

Codecov Report

jdavies-st left a comment • edited Loading

Choose a reason for hiding this comment

jdavies-st commented May 27, 2021

hbushouse left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jdavies-st commented Jul 20, 2021

AL-552: Tests and step code needed to change to accommodate for new parameter in `ramp_fit`. #6072

AL-552: Tests and step code needed to change to accommodate for new parameter in `ramp_fit`. #6072

kmacdonald-stsci commented May 25, 2021 •

edited by jdavies-st

Loading

codecov bot commented May 27, 2021 •

edited

Loading

jdavies-st left a comment •

edited

Loading