search

cta-observatory / ctapipe

Low-level data processing pipeline software for CTAO or similar arrays of Imaging Atmospheric Cherenkov Telescopes
https://ctapipe.readthedocs.org
BSD 3-Clause "New" or "Revised" License
64 stars 268 forks source link

Update ImPACT code to work again #2305

Closed Tobychev closed 6 months ago

Tobychev commented 1 year ago

As requested by @kosack I open a PR with my version of @ParsonsRD ctapipe ImPACT implementation, where I've introduced changes to make the code able to merge with the current HEAD.

This branch also has a modification to some reco tests, and in particular test_reconstruction_changes.py expects that you have a local copy of LSTCam.template.gz and NectarCam.template.gz that I've generated from a larger template file supplied by @ParsonsRD, both should now be available on the test server.

The events used for the test are very low energy events, and the NectarCam template is actually just the LST one again, so performance on these events is not stellar, but they are processes without crashes.

gschwefer commented 1 year ago

I added an implementation of the energy seed. Since this is not strictly necessary to run ImPACT, it is a bit different from the geometry seed. Also, the telescope impact distances were not properly saved in case the reconstruction ran through, so I added the line for that.

gschwefer commented 1 year ago

After discussion with @ParsonsRD , I removed the option for energy preminimization in case no energy seed is present for a given event. An energy seed is now required to run ImPACT.

gschwefer commented 1 year ago

This update mostly includes the changes and fixes currently under discussion in #2388 adapted for the ImPACT pixel likelihood interface. As an aside, keeping constants in the likelihood can actually be important here, namely when likelihoods from the image and time gradient fits are added to get the correct relative statistical weights.

maxnoe commented 7 months ago

Some minor issue in the lint, please fix

maxnoe commented 6 months ago

Hi all,

I am going to merge this now, thanks to everyone who put in the hard work to make this function again! @gschwefer @Tobychev @ParsonsRD

There are a lot of comments for improvements here, but these are very hard to review, so I will merge here and let's continue in easier to review follow-up PRs.