Flat field #161
Flat field #161
Conversation
There was a problem hiding this comment.
Hi @ArmelleJB !
Many thanks for this PR. I'll mark it as draft for the time being, as I understand it is not yet complete.
At some point, in your user script, could you provide an example on how to launch a flatfield computation please ?
Could you also please run pre-commit hooks and commit the linted code ?
|
Hi! The PR #163 has been merged, which results in a couple of small conflicts in this one that I'll try to resolve. |
|
Note that I had to rebase this branch from |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #161 +/- ##
===========================================
+ Coverage 40.76% 63.03% +22.26%
===========================================
Files 65 66 +1
Lines 4452 4829 +377
===========================================
+ Hits 1815 3044 +1229
+ Misses 2637 1785 -852 ☔ View full report in Codecov by Sentry. |
| wfs_pedsub = self.subtract_pedestal(wfs, 20) | ||
|
|
||
| # get the masked array for integration window | ||
| t_peak = np.argmax(wfs_pedsub, axis=3) |
There was a problem hiding this comment.
Just a naive question here: if I am correct, you are manually computing the charge here as what the GlobalPeakWindowSum from ctapipe would do (or LocalPeakWindowSum ?). Why wouldn't we use the ctapipe implementation here? Or maybe I am missing something.
There was a problem hiding this comment.
No you're right, of course we can use that - it's a left over from my original code, I didn't think about what is already implemented in ctapipe.
There was a problem hiding this comment.
No worry at all! Actually, I am never clear whether we should use a global or local charge integrator for the flat field estimation.
To be checked, but it may even be that the ctapipe charge extractors have a correction option when the pulses are on the edges of the time window, as we discussed the last couple of days.
|
|
||
| # get integrated amplitude and mean amplitude over all pixels per event | ||
| amp_int_per_pix_per_event = np.sum( | ||
| wfs_pedsub[0], axis=2, where=masked_wfs.astype("bool") |
There was a problem hiding this comment.
I guess "0" here refers to the high gain channel, is that right ? In such a case, it would be better to use ctapipe_io_nectarcam.constants.HIGH_GAIN instead, in order to avoid hardcoding stuff.
There was a problem hiding this comment.
Ah, actually no, this zero was calling the first event (and there is always only one event in wfs and wfs_pedsum) - I fixed that.
…ield from list to np ndarray
…to calculate gain, hi/lo ratio and identify bad pixels
| amp_int_per_pix_per_event = np.sum( | ||
| wfs_pedsub[0], axis=2, where=masked_wfs.astype("bool") | ||
| ) | ||
| amp_int_per_pix_per_event = np.sum(wfs_pedsub, axis=-1, where=masked_wfs) |
There was a problem hiding this comment.
Why not keeping where=masked_wfs.astype("bool") here, to ensure masked_wfs is a boolean array ?
There was a problem hiding this comment.
Yes we can ! I removed it because I now define the mask array as boolean, but it doesn't hurt to keep it here.
Add component, container and tool for calculation of flat field coefficients.