WIP: Add gain selection to r1 calibration #683
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
first step: label the waveform indices where possible to avoid confusion later
- removed config option `sig_amp_cut_HG, sig_amp_cut_LG` and replaced with a single `peak_detection_threshold` - modified routines to ignore channel (which was formerly axis=0) - labeled the axes explicitly using the `WaveAxis` class. - note: still seems to return a 3D waveform (where `waveform[0]==waveform[1]`), and peak detection stopped working, so still need some work
the inputs were not quite right
…nto add_gain_selection_to_r1cal
…nto add_gain_selection_to_r1cal
…nto add_gain_selection_to_r1cal
…nto add_gain_selection_to_r1cal
…nto add_gain_selection_to_r1cal
Recreate changes for add_gain_selection_to_r1cal
|
Something further may need to be considered for the integration_correction in dl1.py. The MC file may have a different reference pulse shape depending on the channel, and therefore a different integration correction is needed. In the current version of this PR, you simply choose the first channel's integration correction. |
Closed
…nto add_gain_selection_to_r1cal
…nto add_gain_selection_to_r1cal
…ck/ctapipe into add_gain_selection_to_r1cal
…nto add_gain_selection_to_r1cal
…nto add_gain_selection_to_r1cal
24 tasks
|
closed this since the calib refactoring will replace it |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I've attempted to make the data flow a bit more like what CTA is envisioning, where at the R1 level the gain-channel selection has already been performed. For that the following change were make:
Main changes
modified
HESSIOR1Calibratorto include a GainSelector (threshold by default). I didn't know the correct gain-switch thresholds for most cameras, so put some dummy values inctapipe-extra. Those will need to be updated by expertsModified all
ChargeExtractorsto assume a single gain channel (this simplified them a lot), and changed the options called sig_amp_cut_LG and sig_amp_cut_HG to a single option called peak_detection_threshold (which I think is more understandable)I changed the name of the
cleanedField inR1CameraContainerto bewaveformas before (it now contains the gain-selected, cleaned, calibrated waveform)added
waveform_fullandgain_channelmask to the R1Container, for times when both channels are still needed.updated the tests to expect a single gain past R1
Things that do not work so far
This has broken the
TargetIOR1CameraCalibrator, which still expects 2 gains (i didn't want to touch that since I'm not sure exactly how it works. This is probably the same for theSST1MR1CameraCalibrator, so I'll need some help with those.same for the
CHECMWaveFormCleaner- it breaks, since the input from the corresponding CameraCalibrator is still 2-channel.