Kgain e2e

[JuergenSchreiber]

Describe your changes

• added an end-to-end test for kgain determination inclusive the corresponding walker recipe
• compare the determined kgain and readnoise with the ones determined from TVAC

Type of change

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

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

Checklist before requesting a review

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

[JuergenSchreiber]

For efficiency reasons I have only used a subset of the available TVAC noise data, so I had to reduce the n_cal and n_mean parameters to 3, I think I should change that back to default for the real e2e tests?

[semaphoreP]

What is the difference in efficiency? We ideally want to test against a realistic dataset, which includes assessing the computing resources we need. Note that we don't intend to run the e2e tests against every PR like we do for the unit tests.

[JuergenSchreiber]

Results from e2e test with all files using default parameters of calibrate_kgain: determined kgain: 9.394962110962286 determined read noise 146.54419532831497 difference to TVAC kgain: 0.6949621109622868 difference to TVAC read noise: 13.54419532831497 error of kgain: [1.10248111] error of readnoise: 18.162370531395826

Any thoughts on the quite high difference to TVAC?

[semaphoreP]

We recently learned that the Kgain and Nonlin algorithms were updated after the TVAC data products were reduced, and the version we ported over is the updated version. This means that the TVAC data products were generated with the old code unfortunately (this is what Max is referring to in his recent comments in #146). So we have to unfortunately do an extra step: run the TVAC data through the updated engineering pipeline and compare it with our ported version (for nonlin, Sergi confirmed this made the differences disappear). Can you do that? Hopefully, it won't take too long to get running. We can then store the output products on Box for this e2e test to compare against.

Do you have a copy of the original Kgain code from the updated engineering pipeline? If not, @mygouf is working on releasing it, so you should contact her for the latest status.

[mygouf]

We recently learned that the Kgain and Nonlin algorithms were updated after the TVAC data products were reduced, and the version we ported over is the updated version. This means that the TVAC data products were generated with the old code unfortunately (this is what Max is referring to in his recent comments in #146). So we have to unfortunately do an extra step: run the TVAC data through the updated engineering pipeline and compare it with our ported version (for nonlin, Sergi confirmed this made the differences disappear). Can you do that? Hopefully, it won't take too long to get running. We can then store the output products on Box for this e2e test to compare against.

Do you have a copy of the original Kgain code from the updated engineering pipeline? If not, @mygouf is working on releasing it, so you should contact her for the latest status.

kgain code is now released and available here: https://github.com/roman-corgi/cgi_iit_drp/tree/main/Calibration_NTR/cal/kgain

[JuergenSchreiber]

@mygouf, thanks. Do I have to take the same ROI as @hsergi did for nonlin e2e? OK, the ROI is defined in the kgainparams, but the offset"xxxroi" is not applied.

[maxwellmb]

@mygouf, thanks. Do I have to take the same ROI as @hsergi did for nonlin e2e? OK, the ROI is defined in the kgainparams, but the offset"xxxroi" is not applied.

I think you just want to reproduce as much as possible what the KGain TVAC code is doing, not necessarily what nonlin is doing, so you want to the right ROI from the TVAC KGain code.

[JuergenSchreiber]

OK, now it can be reviewed. The test results and performance I have put on confluence