Skip to content

Conversation

maxnoe
Copy link
Member

@maxnoe maxnoe commented Sep 4, 2025

Continuation of #214, due to the large changes in rebase, I didn't want to force-push over that branch but leave it for reference.

@maxnoe maxnoe force-pushed the iter_all_events_2025 branch from 310c8c4 to 48d7ca7 Compare September 5, 2025 07:40
@maxnoe maxnoe marked this pull request as draft September 5, 2025 10:15
@maxnoe maxnoe force-pushed the iter_all_events_2025 branch from 7318873 to 1784b4e Compare September 5, 2025 10:49
Improve performance by removing the need to "peek" to the next object
before parsing it by putting event building logic directly into
`_parse_next_low_level`.
@maxnoe maxnoe marked this pull request as ready for review September 8, 2025 11:59
@maxnoe maxnoe requested review from orelgueta and kosack September 8, 2025 11:59
@maxnoe
Copy link
Member Author

maxnoe commented Sep 8, 2025

In the changes in 3e67777 I managed to remove most of the performance overhead the initial changes of this branch introduced.

@maxnoe maxnoe changed the title Iter all events 2025 Refactor SimTelFile to allow iterating over all events based Sep 8, 2025
@maxnoe maxnoe mentioned this pull request Sep 8, 2025
Copy link
Contributor

@orelgueta orelgueta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change look good, thanks! I had just one small comment on a test.

Also, I did not go over the assumed eventio structure in detail. If you would like me to do so, please let me know.

I will approve once both of these points are cleared up.

@maxnoe
Copy link
Member Author

maxnoe commented Sep 8, 2025

And here are the corresponding changes needed in ctapipe to support iterating over also the non-triggered events:
cta-observatory/ctapipe#2835

@orelgueta
Copy link
Contributor

In case it was lost before: If you don't need me to look in detail at the assumed eventio structure, I can approve already.

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

Successfully merging this pull request may close these issues.

2 participants