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

Chunk interpolation to select calibration data #2634

Open
wants to merge 31 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
3d879b9
Adding the chunk function
Oct 29, 2024
c2ce42c
Reverting to the previous implementation
Oct 29, 2024
c8f7ef0
Changing some variables
Oct 29, 2024
bef9677
Chaning to using scipy functions
Oct 30, 2024
cd4864f
fixing docustring
Oct 30, 2024
fb3a9a4
Updating docustrings further
Oct 30, 2024
0f0b948
simplifying chunk interpolation
Oct 31, 2024
50f2791
Refactor ChunkInterpolator and its tests
mexanick Oct 31, 2024
90115d6
add back base Interpolator to __all__
mexanick Oct 31, 2024
f772b19
adding changelog
Oct 31, 2024
e201356
Merge branch 'ChunkFunction' of https://github.com/cta-observatory/ct…
Oct 31, 2024
907b6cd
renaming changelog
Oct 31, 2024
dc3b24c
Changing inheritance scheme
Nov 22, 2024
3c7e6ab
reverting some tests
Nov 22, 2024
cad523e
documentation change
Nov 22, 2024
4a032ed
fixing issue with class variable
Nov 25, 2024
7ae62e2
implementing reviewer comment
Nov 27, 2024
2997b6c
moving some instance variales to class variables
Dec 2, 2024
a6be0ae
removing unnecessary class variables in parent classes
Dec 3, 2024
8498274
moving ChunkInterpolator variables and making them mutable at first
Dec 4, 2024
a915d8d
removing provate data definition from parent class
Dec 4, 2024
3423bd3
moving a variable
Dec 4, 2024
496af8f
putting required units on ChunkInterpolator
Dec 4, 2024
3b93770
implementing reviewer comment
Dec 4, 2024
da86bf6
making required_columns an instance variable
Dec 9, 2024
50b9e84
making subclasses to ChunkInterpolator
Dec 9, 2024
c11b6f2
simplifying start_time and end_time
Dec 9, 2024
b02b526
adding data groups
Dec 10, 2024
16aa176
adding child classes, making chunk function take arrays
Dec 12, 2024
e0c0004
making the nan switch test check if the switch is done element-wise
Dec 12, 2024
1644388
adding imports to __init__
Dec 13, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/changes/2634.feature.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add ChunkInterpolator to ctapipe.monitoring.interpolation as a tool to select data from chunks. The planned use for this is to select calibration data.
11 changes: 9 additions & 2 deletions src/ctapipe/monitoring/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,12 @@
Module for handling monitoring data.
"""
from .aggregator import PlainAggregator, SigmaClippingAggregator, StatisticsAggregator
from .interpolation import Interpolator, PointingInterpolator
from .interpolation import (
ChunkInterpolator,
ctoennis marked this conversation as resolved.
Show resolved Hide resolved
LinearInterpolator,
MonitoringInterpolator,
PointingInterpolator,
)
from .outlier import (
MedianOutlierDetector,
OutlierDetector,
Expand All @@ -18,6 +23,8 @@
"RangeOutlierDetector",
"MedianOutlierDetector",
"StdOutlierDetector",
"Interpolator",
"MonitoringInterpolator",
"LinearInterpolator",
"PointingInterpolator",
"ChunkInterpolator",
]
263 changes: 211 additions & 52 deletions src/ctapipe/monitoring/interpolation.py
Original file line number Diff line number Diff line change
@@ -1,85 +1,76 @@
from abc import ABCMeta, abstractmethod
from functools import partial
from typing import Any

import astropy.units as u
import numpy as np
import tables
from astropy.table import Table
from astropy.time import Time
from scipy.interpolate import interp1d

from ctapipe.core import Component, traits

__all__ = [
"Interpolator",
"MonitoringInterpolator",
"LinearInterpolator",
"PointingInterpolator",
"ChunkInterpolator",
]


class Interpolator(Component, metaclass=ABCMeta):
class MonitoringInterpolator(Component, metaclass=ABCMeta):
"""
Interpolator parent class.
MonitoringInterpolator parent class.

Parameters
----------
h5file : None | tables.File
A open hdf5 file with read access.
An open hdf5 file with read access.
"""

bounds_error = traits.Bool(
default_value=True,
help="If true, raises an exception when trying to extrapolate out of the given table",
).tag(config=True)

extrapolate = traits.Bool(
help="If bounds_error is False, this flag will specify whether values outside"
"the available values are filled with nan (False) or extrapolated (True).",
default_value=False,
).tag(config=True)

telescope_data_group = None
required_columns = set()
expected_units = {}

def __init__(self, h5file=None, **kwargs):
def __init__(self, h5file: None | tables.File = None, **kwargs: Any) -> None:
super().__init__(**kwargs)

if h5file is not None and not isinstance(h5file, tables.File):
raise TypeError("h5file must be a tables.File")
self.h5file = h5file

self.interp_options: dict[str, Any] = dict(assume_sorted=True, copy=False)
if self.bounds_error:
self.interp_options["bounds_error"] = True
elif self.extrapolate:
self.interp_options["bounds_error"] = False
self.interp_options["fill_value"] = "extrapolate"
else:
self.interp_options["bounds_error"] = False
self.interp_options["fill_value"] = np.nan
@abstractmethod
def __call__(self, tel_id: int, time: Time):
"""
Interpolates monitoring data for a given timestamp

self._interpolators = {}
Parameters
----------
tel_id : int
Telescope id.
time : astropy.time.Time
Time for which to interpolate the monitoring data.

"""
pass

@abstractmethod
def add_table(self, tel_id, input_table):
def add_table(self, tel_id: int, input_table: Table) -> None:
"""
Add a table to this interpolator
Add a table to this interpolator.
This method reads input tables and creates instances of the needed interpolators
to be added to _interpolators. The first index of _interpolators needs to be
tel_id, the second needs to be the name of the parameter that is to be interpolated
tel_id, the second needs to be the name of the parameter that is to be interpolated.

Parameters
----------
tel_id : int
Telescope id
Telescope id.
input_table : astropy.table.Table
Table of pointing values, expected columns
are always ``time`` as ``Time`` column and
other columns for the data that is to be interpolated
other columns for the data that is to be interpolated.
"""

pass

def _check_tables(self, input_table):
def _check_tables(self, input_table: Table) -> None:
missing = self.required_columns - set(input_table.colnames)
if len(missing) > 0:
raise ValueError(f"Table is missing required column(s): {missing}")
Expand All @@ -98,14 +89,14 @@ def _check_tables(self, input_table):
f"{col} must have units compatible with '{self.expected_units[col].name}'"
)

def _check_interpolators(self, tel_id):
def _check_interpolators(self, tel_id: int) -> None:
if tel_id not in self._interpolators:
if self.h5file is not None:
self._read_parameter_table(tel_id) # might need to be removed
else:
raise KeyError(f"No table available for tel_id {tel_id}")

def _read_parameter_table(self, tel_id):
def _read_parameter_table(self, tel_id: int) -> None:
# prevent circular import between io and monitoring
from ..io import read_table

Expand All @@ -116,32 +107,67 @@ def _read_parameter_table(self, tel_id):
self.add_table(tel_id, input_table)


class PointingInterpolator(Interpolator):
class LinearInterpolator(MonitoringInterpolator):
"""
LinearInterpolator parent class.

Parameters
----------
h5file : None | tables.File
An open hdf5 file with read access.
"""

bounds_error = traits.Bool(
default_value=True,
help="If true, raises an exception when trying to extrapolate out of the given table",
).tag(config=True)

extrapolate = traits.Bool(
help="If bounds_error is False, this flag will specify whether values outside"
"the available values are filled with nan (False) or extrapolated (True).",
default_value=False,
).tag(config=True)

def __init__(self, h5file: None | tables.File = None, **kwargs: Any) -> None:
super().__init__(h5file, **kwargs)
self._interpolators = {}
self.interp_options: dict[str, Any] = dict(assume_sorted=True, copy=False)
if self.bounds_error:
self.interp_options["bounds_error"] = True
elif self.extrapolate:
self.interp_options["bounds_error"] = False
self.interp_options["fill_value"] = "extrapolate"
else:
self.interp_options["bounds_error"] = False
self.interp_options["fill_value"] = np.nan


class PointingInterpolator(LinearInterpolator):
Copy link
Member

Choose a reason for hiding this comment

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

Same here: the only change for PointingInterpolator should be the base class

"""
Interpolator for pointing and pointing correction data
Interpolator for pointing and pointing correction data.
"""

telescope_data_group = "/dl0/monitoring/telescope/pointing"
required_columns = frozenset(["time", "azimuth", "altitude"])
expected_units = {"azimuth": u.rad, "altitude": u.rad}

def __call__(self, tel_id, time):
def __call__(self, tel_id: int, time: Time) -> tuple[u.Quantity, u.Quantity]:
"""
Interpolate alt/az for given time and tel_id.

Parameters
----------
tel_id : int
telescope id
Telescope id.
time : astropy.time.Time
time for which to interpolate the pointing
Time for which to interpolate the pointing.

Returns
-------
altitude : astropy.units.Quantity[deg]
interpolated altitude angle
Interpolated altitude angle.
azimuth : astropy.units.Quantity[deg]
interpolated azimuth angle
Interpolated azimuth angle.
"""

self._check_interpolators(tel_id)
Expand All @@ -151,20 +177,19 @@ def __call__(self, tel_id, time):
alt = u.Quantity(self._interpolators[tel_id]["alt"](mjd), u.rad, copy=False)
return alt, az

def add_table(self, tel_id, input_table):
def add_table(self, tel_id: int, input_table: Table) -> None:
"""
Add a table to this interpolator
Add a table to this interpolator.

Parameters
----------
tel_id : int
Telescope id
Telescope id.
input_table : astropy.table.Table
Table of pointing values, expected columns
are ``time`` as ``Time`` column, ``azimuth`` and ``altitude``
as quantity columns for pointing and pointing correction data.
"""

self._check_tables(input_table)

if not isinstance(input_table["time"], Time):
Expand All @@ -184,5 +209,139 @@ def add_table(self, tel_id, input_table):
alt = input_table["altitude"].quantity.to_value(u.rad)
mjd = input_table["time"].tai.mjd
self._interpolators[tel_id] = {}
self._interpolators[tel_id]["az"] = interp1d(mjd, az, **self.interp_options)
self._interpolators[tel_id]["alt"] = interp1d(mjd, alt, **self.interp_options)
self._interpolators[tel_id]["az"] = interp1d(
mjd, az, kind="linear", **self.interp_options
)
self._interpolators[tel_id]["alt"] = interp1d(
mjd, alt, kind="linear", **self.interp_options
)


class ChunkInterpolator(MonitoringInterpolator):
"""
Simple interpolator for overlapping chunks of data.
"""

def __init__(self, h5file: None | tables.File = None, **kwargs: Any) -> None:
super().__init__(**kwargs)
self._interpolators = {}
self.required_columns = ["start_time", "end_time"]
Copy link
Contributor

Choose a reason for hiding this comment

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

required columns as start and stop time shall be the class attributes and shall be frozen, they are mandatory. You can copy them to an instance and extend with a value column(s) when you engage a __call__.

Copy link
Author

Choose a reason for hiding this comment

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

as the functions that use required_columns are in the parent class and always look to that name it makes more sense to have it as a modifiable instance variable

Copy link
Member

Choose a reason for hiding this comment

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

No. The original design idea was to have the required_columns frozen per "final" class. I.e. MonitoringInterpolator requires altitude and azimuth columns.

This is due to the configuration system in ctapipe, which works on class name basis, not instances.

Copy link
Author

Choose a reason for hiding this comment

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

So, the most sensible way to keep the required units and columns as class variables is to have pedestal and flatfield interpolators that inherit from the ChunkInterpolator. The ChunkInterpolator now has no required columns or units, but rather the subclasses have those variables. If we use the FlatFieldInterpolator we know we will always look for a column with relative gain factors with no unit, similarly we know what data a PedestalInterpolator will need.

I made those classes, and if we want to use the Chunk interpolation later for some other data we can add another subclass.

self.expected_units = {}
Copy link
Member

Choose a reason for hiding this comment

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

Why are these instance variables? And why are they empty?

This is not in line with how the other class works.

It seems this class variable is unused even.

Copy link
Contributor

Choose a reason for hiding this comment

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

Here required_columns shall become class attributes, but the interpolators and expected units shall remain instance variable, to allow creation of multiple instances. We have to see in the future PR, whether we want further specialization (e.g. a factory of VarNameInterpolators), that may lead to change this (they will basically become singletons).

Copy link
Contributor

Choose a reason for hiding this comment

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

expected units are unused because the quantity is dimensionless, we may want to actually enforce this through u.dimensioneless_unscaled

Copy link
Author

Choose a reason for hiding this comment

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

We had implemented that for collumns that are supposed to not have a unit we set the expected unit to None and check if the actual unit is equivalent. I put it like that.

self.start_time = {}
self.end_time = {}
self.values = {}

def __call__(
self, tel_id: int, time: Time, columns: str | list[str]
) -> float | dict[str, float]:
"""
Interpolate overlapping chunks of data for a given time, tel_id, and column(s).

Parameters
----------
tel_id : int
Telescope id.
time : astropy.time.Time
Time for which to interpolate the data.
columns : str or list of str
Name(s) of the column(s) to interpolate.

Returns
-------
interpolated : float or dict
Interpolated data for the specified column(s).
"""

self._check_interpolators(tel_id)

if isinstance(columns, str):
columns = [columns]

result = {}
mjd = time.to_value("mjd")
for column in columns:
if column not in self._interpolators[tel_id]:
raise ValueError(
f"Column '{column}' not found in interpolators for tel_id {tel_id}"
)
result[column] = self._interpolators[tel_id][column](mjd)
Copy link
Member

Choose a reason for hiding this comment

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

Why use self._interpolators, why keep that around at all?

Why not just call self._interpolate_chunk(tel_id, column, mjd)?

Copy link
Author

Choose a reason for hiding this comment

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

read_table checks if _interpolators has already been set up for the given tel_id. _check_interpolators in MonitoringInterpolator also does that check and adds data from hdf5file if the interpolator is not set. I can move column to be an argument of _interpolate_chunk though.


if len(result) == 1:
return result[columns[0]]
return result

def add_table(self, tel_id: int, input_table: Table, columns: list[str]) -> None:
"""
Add a table to this interpolator for specific columns.

Parameters
----------
tel_id : int
Telescope id.
input_table : astropy.table.Table
Table of values to be interpolated, expected columns
are ``start_time`` as ``validity start Time`` column,
``end_time`` as ``validity end Time`` and the specified columns
for the data of the chunks.
columns : list of str
Names of the columns to interpolate.
"""

required_columns = set(self.required_columns)
required_columns.update(columns)
self.required_columns = frozenset(required_columns)
self._check_tables(input_table)

input_table = input_table.copy()
input_table.sort("start_time")

if tel_id not in self._interpolators:
self._interpolators[tel_id] = {}
self.values[tel_id] = {}
self.start_time[tel_id] = {}
self.end_time[tel_id] = {}

for column in columns:
self.values[tel_id][column] = input_table[column]
self.start_time[tel_id][column] = input_table["start_time"].to_value("mjd")
Copy link
Member

Choose a reason for hiding this comment

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

Why store start_time and end_time per column?

self.end_time[tel_id][column] = input_table["end_time"].to_value("mjd")
self._interpolators[tel_id][column] = partial(
self._interpolate_chunk, tel_id, column
)

def _interpolate_chunk(self, tel_id, column, mjd: float) -> float:
"""
Interpolates overlapping chunks of data preferring earlier chunks if valid

Parameters
----------
tel_id : int
tel_id for which data is to be interpolated
column : str
name of the column for which data is to be interpolated
mjd : float
Time for which to interpolate the data.
"""

start_time = self.start_time[tel_id][column]
end_time = self.end_time[tel_id][column]
values = self.values[tel_id][column]
# Find the index of the closest preceding start time
preceding_index = np.searchsorted(start_time, mjd, side="right") - 1
if preceding_index < 0:
return np.nan

# Check if the time is within the valid range of the chunk
if start_time[preceding_index] <= mjd <= end_time[preceding_index]:
value = values[preceding_index]
if not np.isnan(value):
return value

# If the closest preceding chunk has nan, check the next closest chunk
for i in range(preceding_index - 1, -1, -1):
if start_time[i] <= mjd <= end_time[i]:
value = values[i]
if not np.isnan(value):
return value

return np.nan
Loading
Loading