Cosmics kgain

[JuergenSchreiber]

Added KGain calibration file as mandatory input parameter in detect_cosmic_rays

Type of change

• New feature (non-breaking change which adds functionality)

Reference any relevant issues (don't forget the #)

203

Checklist before requesting a review

• [ ] I have linted my code
• [ ] I have verified that all unit tests pass in a clean environment and added new unit tests, as needed
• [ ] I have verified that all docstrings are properly formatted and added new documentation, as needed

[JuergenSchreiber]

I guess I still have to update all the e2e tests with a kgain caldb entry where detect_cosmic_rays is used...

[semaphoreP]

Can this be an optional calibration? since detect cosmic rays is a L2a level step, while Kgain is a L2b level calibration product

[kjl0025]

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 detect_cosmic_rays, which would be circular. So I'm sorry to say this given all your work on this, Juergen, but I think we should actually cancel this PR and leave the code as it is (drawing k gain from DetectorParams), and we would just update the DetectorParams default k gain value whenever a new k gain calibration is done. Jason can chime in if he has other thoughts.

[maxwellmb]

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?

[semaphoreP]

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.

[kjl0025]

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?

Yes, that is correct.

And I agree with Jason's proposal. That's probably better than having to update DetectorParams after a calibration.

[maxwellmb]

I don't think I understand why we have a KGain in DetectorParams. What is its relation to the KGain calibration. A backup?

[kjl0025]

I think we put it in there initially, before we had started on the calibration script for k gain.

[JuergenSchreiber]

@maxwellmb: yes, kgain in detetor_params works like a backup if I implement it like desired.

[JuergenSchreiber]

To make this work as first parameter, I have to specify detector_params also as an optional parameter