Skip to content

Add a few containers for monitoring data - #3063

Open
mexanick wants to merge 12 commits into
mainfrom
add-monitoring-containers
Open

Add a few containers for monitoring data#3063
mexanick wants to merge 12 commits into
mainfrom
add-monitoring-containers

Conversation

@mexanick

@mexanick mexanick commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Containers implementation for #3055

Comment thread src/ctapipe/containers.py Outdated
@maxnoe

maxnoe commented Aug 4, 2026

Copy link
Copy Markdown
Member

I'm not sure that having the containers with nan defaults in the event structure is the right choice in this case. Maybe better use default=None, type=TelescopeStructurePointingContainer for the fields in the event?

Like this, it's clear that the monitoring was not filled at all, not that we tried to fill it and didn't succeed.

@mexanick
mexanick marked this pull request as ready for review August 5, 2026 07:06
Comment thread src/ctapipe/containers.py Outdated
Comment thread src/ctapipe/containers.py Outdated
maxnoe
maxnoe previously approved these changes Aug 5, 2026
@mexanick
mexanick requested review from Voutsi and kosack August 5, 2026 13:38
Comment thread src/ctapipe/containers.py Outdated
Comment thread src/ctapipe/containers.py Outdated
Comment thread src/ctapipe/containers.py
Comment thread src/ctapipe/containers.py
Comment thread src/ctapipe/containers.py Outdated
@mexanick
mexanick requested review from Voutsi and maxnoe August 11, 2026 11:39
Comment thread src/ctapipe/containers.py Outdated
Comment thread src/ctapipe/containers.py Outdated
Comment thread src/ctapipe/containers.py Outdated
default_factory=CameraMonitoringContainer,
description="Container for monitoring data for camera",
)
structure_pointing = Field(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Following our discussion on slack on when / how lower-level monitoring is combined, I don't really see the need to add these containers to the event structure.

Is there a use case remaining?

I think we agreed that the more efficient approach is to compute the final correction on the monitoring time scales based on tabular data and interpolate that to the event.

In that scenario, we would not need event-wise data structures for these lower-level parts.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think we still want them for the star tracking. However, in this case I should leave them as standalone containers, but remove from being linked to the event, so it is only filled on request and not for every event processed with ctapipe-process tool, right?

In this case, I will leave TelescopePointing and CameraDisplacement containers here (I believe there's no easy way around for Camera Displacement, as I believe it would be only used to modify image, so we just take it as is and interpolate to the event time).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You mean as output containers from the star tracking which would work on bunches of events? Or as intermediate input per event?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

as intermediate event-wise input to star tracker. The output of star tracker would be a grid of corrections (common grid with the structure monitoring).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why would the star-tracker not use the pointing filled from Structure + BendingModel into the telescope poiinting container?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It will use the pointing from Structure + BendingModel as the seed, but it will also use CDM for a given event and weather values for the given event.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sure, weather is needed and should be added here.

But for the pointing, I would think that you configure the DL0MonitoringSource to read Structure, BendingModel and CDM files and use that to fill the already existing pointing container.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, it will be done like that. However, I still need to define the TelescopeStructurePointingContainer, TelescopeStructureDisplacementContainer, etc for conversion, to abstract telescope-specific implementations. They, however, would not be added to the event monitoring structure.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't understand for what you need the containers then

@ctao-sonarqube

Copy link
Copy Markdown

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.

3 participants