Flashcam extractor

[clara-escanuela]

No description provided.

[maxnoe]

Hi @clara-escanuela

In your PR the CI is failing, at least with the static code checks (some style issues).

We recently also updated the way these checks are done, so please rebase the current main branch and fix the issues

[maxnoe]

The code looks mostly fine now, I will have a deeper look later, however let me note that this is now much (factor of six for a La Palma simulation processing all telescopes, not just the flashcams) slower than the defaul NeighborPeakWindow extractor, which itself is already some factor slower than e.g. the LocalPeakWindow extractor, due to the overhead of look at pixel neighbors.

I didn't do a profiling, but before we discussed that the performance of scipy filtfilt is suboptimal as it contains basically a pure python loop over the outer dimensions, also see here:

scipy/scipy#17080

Since if I read the code correctly, the actual filter used is not that complicated, would it be possible to use numba here to implement it?

Of course code has first to be correct, second be fast, but I think it should be relatively simple to get decent performance here.

[clara-escanuela]

@maxnoe this implementation is in very good agreement with filtfilt and is way faster. You could check again the speed, should be better. The charge resolution does not change at all, so the upsampling is good. Although, maybe the function should be defined in another file where some other general tools are also implemented, so that people can have use it for upsampling.

[maxnoe]

This looks great, and with the latest changes it also got a lot faster.

Checking a north alpha run with only flashcams in allowed_tels I get:

FlashCam:      00:54, 17.14ev/s
NeighborPeak : 00:21, 44.24ev/s

Which seems like what we can expect since the flashcam extractor is essentially neighbor peak with addional steps. So for me this looks good enough to merge this here and further optimizations if possible can come in a follow up PR.

Just a last minor comment on the new filtfilt function and I think we are ready to go.

Thanks a lot for the great work!

[maxnoe]

Sorry, missed that the one comment was not addressed yet

ABout the guvectorize dtype order

[kosack]

This looks very good! I don't see any issues with the code, and it seems it's well-covered by unit tests.

I did a few ad-hoc tests with some data as well as look at images with both NeighborPeak and FlashCamExtractor, and everything looks reasonable.

One can see clearly that the background is now reduced significantly while the signal is maintained over what is given by a naive application of NeighborPeak to the FlashCam events.

[maxnoe]

@kosack sorry, I forgot to post the plots, I did some similar tests and came to the same conclusion.

[maxnoe]

Y-shaped shower in the second example? @jsitarek :D

[clara-escanuela]

You can also see that here,