-
Notifications
You must be signed in to change notification settings - Fork 270
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
WIP: Add gain selection to r1 calibration #683
Closed
kosack
wants to merge
65
commits into
cta-observatory:master
from
kosack:add_gain_selection_to_r1cal
Closed
Changes from 28 commits
Commits
Show all changes
65 commits
Select commit
Hold shift + click to select a range
2fde044
fix bug when selecting a 2 gain 1 sample waveform
kosack 1109b6c
start to clean up r1.py
kosack e91e722
add gain_channel_mask and waveform_full to R1CameraContainer
kosack fe82bae
perform gain selection at the R1 level
kosack a1002ed
add __init__ to GainSelector so NullGainSelector works
kosack 5419edc
start to refactor ChargeExtractors to expect a single channel
kosack 21a3e46
assume no channel in waveforms
kosack 0a3b9e9
fix __init__ signature for GainSelector
kosack ea0b864
gain_channel_mask -> gain_channel
kosack a6c6292
start to refactor to expect gain-selected waveform
kosack d0484a1
gain_channel_mask -> gain_channel
kosack 3d55e22
fix call to get_sum_array
kosack 291c50e
ensure gain selection on a 1 sample telescope returns the correct shape
kosack fcd162f
change dl1.tel[x].cleaned -> waveform
kosack 882ef6b
remove comment
kosack 8668774
fixed tests for charge extractors
kosack 70f154c
fix tests for dl0 and r1 to assume gain-selection
kosack 0a9cad8
added a max_events option to simple_pipeline to help with testing
kosack 5d13f15
fix bug where image came out with channel dimension
kosack aaadce0
update some examples
kosack fd45c4c
fix bug handling astri-like (1 sample) images
kosack e0a1eed
update more examples
kosack ccfe1f0
update theata_square example
kosack 8323987
make sure gain_channel is propegated to dl1
kosack 1ab783c
rename data -> event
kosack f454361
update some tests
kosack 9c5ac5a
reformatting
kosack b3896fa
reformatting
kosack 7e6c0c5
reformatting
kosack de85327
reformatting
kosack e7f5b64
make NullR1Calibrator use a SimpleGainSelector by default
kosack a026e91
small refactoring:
kosack 44c7cab
switch to using CAPS for Enum values (following convention)
kosack 27a9cd3
use IntEnum instead of Enum
kosack 438c30e
Changes for Target classes
watsonjj 62e18fb
Merge remote-tracking branch 'remotes/kosack/add_gain_selection_to_r1…
watsonjj e9c6815
Moved default gain_selector to CameraR1Calibrator
watsonjj 06cac30
Merge pull request #2 from watsonjj/add_gain_selection_to_r1cal
kosack 116890a
Revert "Add gain selection to r1cal"
kosack ff14efa
Merge pull request #3 from kosack/revert-2-add_gain_selection_to_r1cal
kosack 83f1004
Recreate changes for add_gain_selection_to_r1cal
watsonjj 99f2250
Merge branch 'master' of https://github.com/cta-observatory/ctapipe i…
kosack 06f6fe7
Merge branch 'master' of https://github.com/cta-observatory/ctapipe i…
kosack a9063e2
Merge branch 'add_gain_selection_to_r1cal' into add_gain_selection_to…
kosack 1d0b906
made calib_scale a config attribute and added more docs
kosack 8befe1f
Merge branch 'master' of https://github.com/cta-observatory/ctapipe i…
kosack c45add2
Merge branch 'master' of https://github.com/cta-observatory/ctapipe i…
kosack 66ab4d5
start to change to using un-calibrated gain switch threshold
kosack 02986f2
Merge branch 'master' of https://github.com/cta-observatory/ctapipe i…
kosack 5c99a58
Merge branch 'master' of https://github.com/cta-observatory/ctapipe i…
kosack 893715d
for now, check for several column names in the threshold file (until …
kosack a61ade8
Merge branch 'master' of https://github.com/cta-observatory/ctapipe i…
kosack 358c9bd
Merge branch 'master' of https://github.com/cta-observatory/ctapipe i…
kosack 599520f
Merge branch 'add_gain_selection_to_r1cal' into add_gain_selection_to…
kosack 0ae2e9b
abstractclassmethod -> abstractmethod
kosack 6cca2de
Merge branch 'watsonjj-add_gain_selection_to_r1cal' into add_gain_sel…
kosack 5544a16
switched order of constructor args to match Component
kosack 8cdf822
Merge branch 'add_gain_selection_to_r1cal' into add_gain_selection_to…
kosack 59a91f9
Merge pull request #4 from watsonjj/add_gain_selection_to_r1cal
kosack 7e7e98d
Merge branch 'master' of https://github.com/cta-observatory/ctapipe i…
kosack facc9ac
Merge branch 'master' of https://github.com/cta-observatory/ctapipe i…
kosack 8b16321
Merge branch 'add_gain_selection_to_r1cal' of https://github.com/kosa…
kosack a0f7582
Merge branch 'master' of https://github.com/cta-observatory/ctapipe i…
kosack 00ff43f
Merge branch 'master' of https://github.com/cta-observatory/ctapipe i…
kosack d166f95
updated example
kosack File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
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
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
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
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
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why use np.squeeze instead of waveforms[0]?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure - that was just copied from Tino's code.. I will change it to
waveforms[0]
since it makes more senseThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just out of curiosity, I see multiple mentions of something called 'Tinos Code'. Is this something one should know about?...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry that may be confusing: Tino (@tino-michael) is the one who created the full pipeline based on ctapipe that has been used so far to make sensitivity curves. To get it working, he had to create a few extra classes and scripts that are in his github repo, and they are not yet merged into ctapipe (this PR is the first step to doing that, but there will be several others once this is working). It turned out to be not as simple as just copying his code in, since we wanted to move things around a bit, like having the gain selection in R1, rather than in Dl1 as it is in his code, hence some refactoring.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I used squeeze and not just the index because at some point there was an inconsistency somewhere with the time dimension. sometimes the whole dimension was gone, sometimes it was there but with length 1...
as in the different behaviour of
a = [1, 2, 3]
andb = [[1, 2, 3]]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that problem with missing time dimensions, etc, has been fixed now in the new EventSource classes and the calibration code... at least I haven't seen it anymore with any of the test data I used.