Cosmics kgain #211
Cosmics kgain #211
Conversation
|
I guess I still have to update all the e2e tests with a kgain caldb entry where detect_cosmic_rays is used... |
|
Can this be an optional calibration? since detect cosmic rays is a L2a level step, while Kgain is a L2b level calibration product |
|
Thanks for this, Juergen, but now that I think about this, to Jason's point, we do have an issue (#202 ) about enhancing the k gain and nonlin code to take into account flagged cosmics to improve k gain and nonlinearity calibration. (The II&T code did not account for cosmic rays, but ideally, that should be done.) So the recipe for making the k gain and nonlin calibrations will eventually use |
|
Do I understand correctly that detect_cosmic_rays needs a KGain value, and its just that right now we're pulling it from DetectorParams, rather than a KGain file? |
|
Can we handle it like how we handle Bias offset subtraction with NoiseMaps in prescan_biassub? We leave it as an optional keyword (it needs to be the first keyword for this to work), and the recipe species Kgain to be "AUTOMATIC,OPTIONAL". Basically, the walker will pass in a Kgain if we have one, or pass in None. If None gets passed in, let's just use the Kgain in the DetectorParams. |
Yes, that is correct. And I agree with Jason's proposal. That's probably better than having to update DetectorParams after a calibration. |
| @@ -207,7 +208,7 @@ def detect_cosmic_rays(input_dataset, detector_params, sat_thresh=0.7, | |||
|
|
|||
|
|
|||
| # Calculate the full well capacity for every frame in the dataset | |||
| kgain = np.array([detector_params.params['kgain'] for frame in crmasked_dataset]) | |||
There was a problem hiding this comment.
If kgain is None, use the value from detector_params. And k_gain would have to be listed before detector_params in the list of arguments so that the "AUTOMATIC, OPTIONAL" works.
There was a problem hiding this comment.
By the way: is there a reason why an array of equal kgain values is made? We can divide an array by a scalar.
There was a problem hiding this comment.
agree, not really needed to have an array since we are not assuming to deal with different kgain values right now
|
I don't think I understand why we have a KGain in DetectorParams. What is its relation to the KGain calibration. A backup? |
|
I think we put it in there initially, before we had started on the calibration script for k gain. |
|
@maxwellmb: yes, kgain in detetor_params works like a backup if I implement it like desired. |
|
To make this work as first parameter, I have to specify detector_params also as an optional parameter |
corgidrp/l1_to_l2a.py
Outdated
| @@ -154,7 +154,7 @@ def prescan_biassub(input_dataset, noise_maps=None, return_full_frame=False, | |||
|
|
|||
| return output_dataset | |||
|
|
|||
| def detect_cosmic_rays(input_dataset, detector_params, k_gain, sat_thresh=0.7, | |||
| def detect_cosmic_rays(input_dataset, k_gain = None, detector_params = None, sat_thresh=0.7, | |||
There was a problem hiding this comment.
sorry, I was a bit unclear here: I meant that kgain should be the first optional keyword (i.e., input_dataset, detector_params, kgain=None, sat_thresh=0.7, ...)
There was a problem hiding this comment.
Understood, changed back again. I think now it should be ready for review.
| @@ -19,7 +19,8 @@ | |||
| { | |||
| "name" : "detect_cosmic_rays", | |||
| "calibs" : { | |||
| "DetectorParams" : "AUTOMATIC" | |||
| "DetectorParams" : "AUTOMATIC", | |||
| "KGain" : "AUTOMATIC, OPTIONAL" | |||
There was a problem hiding this comment.
for this and other recipes below, can the indentation be at the same level?
| # Calculate the full well capacity for every frame in the dataset | ||
| kgain = np.array([detector_params.params['kgain'] for frame in crmasked_dataset]) | ||
| if k_gain == None: |
There was a problem hiding this comment.
should use if k_gain is None (forget the exact reason, but is is preferred to ==)
| @@ -207,7 +208,7 @@ def detect_cosmic_rays(input_dataset, detector_params, sat_thresh=0.7, | |||
|
|
|||
|
|
|||
| # Calculate the full well capacity for every frame in the dataset | |||
| kgain = np.array([detector_params.params['kgain'] for frame in crmasked_dataset]) | |||
There was a problem hiding this comment.
agree, not really needed to have an array since we are not assuming to deal with different kgain values right now
Added KGain calibration file as mandatory input parameter in detect_cosmic_rays
Type of change
Reference any relevant issues (don't forget the #)
#203
Checklist before requesting a review