Skip to content
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

Update code to use more features of ctapipe #357

Open
5 of 18 tasks
maxnoe opened this issue Apr 21, 2020 · 10 comments
Open
5 of 18 tasks

Update code to use more features of ctapipe #357

maxnoe opened this issue Apr 21, 2020 · 10 comments

Comments

@maxnoe
Copy link
Member

maxnoe commented Apr 21, 2020

ctapipe 0.8 is not yet released, but we can collect necessary changes here we already know about.

Some of the points are more general ctapipe adaption and not related to 0.8.
These could be done already, so that the amount of work is less when ctapipe 0.8 comes out.

This is what I came up with, feel free to add stuff I forgot.

And stuff that is new in ctapipe v0.8, but will not be necessary for the functionality of lstchain when moving to v0.8

  • Using TelescopeComponent where appropriate
  • Use Path traitlet instead of Unicode where appropriate
  • Use ctapipe Tool and configuration system for executables. (Could be done now already)
  • Use the new cleaning components
  • Clean up containers (e.g. no dl1-mother-of-all-containers) (DL1 columns #343, DL1 container cleanup #349 )
  • Largest island
  • Use new configurable muon analysis
  • Remove lstchain/image/muon/muon_integrator.py, and use the (fixed in 0.8) ctapipe version
  • Keep all events #237 -> option to choose whether DL1a/b output should include events which did not survive cleaning or not (the former is useful for keeping calibrated pedestal events for e.g. cleaning studies). This was not done until now because we would need to set defaults for all DL1b parameters (not worth considering the container is deprecated)
  • Adapt DL1 data check to the new DL1 containers
@maxnoe maxnoe added this to the ctapipe v0.8 milestone Apr 21, 2020
@rlopezcoto
Copy link
Contributor

What do you mean by Largest island?

@maxnoe
Copy link
Member Author

maxnoe commented Apr 21, 2020

There is now a function in ctapipe that gives you the largest island after cleaning., I think this got implemented here as well and this could be dropped for the ctapipe version.

@moralejo
Copy link
Collaborator

I would say it is better to start addressing this after the next lstchain release, right?
Otherwise the release will be delayed. We would like to have soon a new version of calibrated data to make progress e.g. with cleaning studies.

@rlopezcoto
Copy link
Contributor

There is now a function in ctapipe that gives you the largest island after cleaning., I think this got implemented here as well and this could be dropped for the ctapipe version.

ok, right, this was implemented and we are currently using the main island for the analysis. In any case and after discussing with @jsitarek, for the lowest energies this may not be the best solution (using only the largest island), but that can be studied using Data/MC in the future

@rlopezcoto
Copy link
Contributor

@moralejo I think this is just a proposal to keep track of these points, as discussed yesterday, as soon as we have a code that writes the information of interleaved events and the first datacheck scripts we can proceed with the 0.5 release.

@moralejo
Copy link
Collaborator

@moralejo I think this is just a proposal to keep track of these points, as discussed yesterday, as soon as we have a code that writes the information of interleaved events and the first datacheck scripts we can proceed with the 0.5 release.

Ok, I got nervous with the "could be done already". Fine if already means right after 0.5.0

@maxnoe
Copy link
Member Author

maxnoe commented Apr 21, 2020

I just wanted to note which changes are possible with the current release and which changes are releated to the new ctapipe release.

@moralejo
Copy link
Collaborator

moralejo commented Jun 26, 2020

  • option to choose whether DL1a/b output should include events which did not survive cleaning or not (the former is useful for keeping calibrated pedestal events for e.g. cleaning studies). This was not done until now because we would need to set defaults for all DL1b parameters (not worth considering the container is deprecated)

  • Adapt DL1 data check to the new DL1 containers

  • Remove lstchain/image/muon/muon_integrator.py, and use the (fixed in 0.8) ctapipe version

(@rlopezcoto: already added them to the full list)

@FrancaCassol
Copy link
Collaborator

Hi to all,
to be sure not to have destructive interferences, should we better assign names to the different points in the above list ?
I am taking care of the calibration code and gain selection, but I can take care also of other points.
For example what to you mean for "point container"? The idea is to move it to ctapipe?

@maxnoe
Copy link
Member Author

maxnoe commented Oct 14, 2020

Since lstchain is now already using ctapipe 0.8 this issue is partly solved, but many improvements are not yet implemented.

@maxnoe maxnoe changed the title Switching to ctapipe 0.8 Update code to use more features of ctapipe Oct 14, 2020
@maxnoe maxnoe removed this from the ctapipe v0.8 milestone Oct 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants