simonsobs / simonsobs/sotodlib
Standardized interface for operations on an AxisManager
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 19
- Forks
- 23
- Avg merge
- 1d 5h
- Merged PRs (30d)
- 14
Description
This is a proposal and request-for-comment on the internal interface for low-level TOD processing functions, such as filters, flagging, coordinates & projections, etc. Potentially anything that deals with TOD as stored in an AxisManager.
We are starting to have lots of functions that take "tod", an AxisManager, as their main argument. A function that needs to look at several different vectors (e.g. the detector signals and the timestamps) can grab what it needs from tod (tod.signal and tod.timestamps). But such functions will have limited usefulness if one cannot override those sensible defaults (e.g. we may want to apply a filter to tod.signal_detrended instead of tod.signal). Conversely, the code will be annoying to use if each submodule or function accommodates overrides in a different way.
I propose a partial solution, below.
Parameters to functions
Functions that operate on data taken from an AxisManager will have 3 classes of parameters:
- AxisManagers -- probably only one or two of these; bundles of arrays from which default data for processing will be drawn.
- Processing parameters -- these control some aspect of the processing but do not explicitly refer to data arrays that are inputs for the processing.
- Data overrides -- these are optional, allowing the user to provide an array to stand in for some purpose. The parameter names, here, should be the same as the default vector name that would otherwise be used. For example, if tod.signal would be used by default, then the override argument should be signal=... . Passing None should cause the default data to be used; passing False should cause the specified data to be treated as absent.
In the function signature, parameters should be ordered consistent with the order above -- AxisManagers first (perhaps just a tod), then processing parameters (which need not have defaults), and then data overrides, which presumably default to None.
Rationale for passing override vectors rather than override vector names: It might seem convenient to allow the user to specify an override data vector by name, e.g. do_something(tod, signal='signal_clean') would cause something to be done to tod.signal_clean instead of tod.signal. This is not really less typing than do_something(tod, signal=tod.signal_clean), and yet it requires additional logic in the function to handle both string and array arguments. I think it will lead to clearer code on both sides if we only specify overrides as data arrays, and do not bother with vector names. (Pipeline-style operator functions, where a callable is instantiated with some parameters, but then actually executed on data later, would break this... but those are not what we're talking about.)
Stashing results
In many cases a function will compute new data arrays that might reasonably be stored into one of the AxisManagers that was provided as an argument. It's a hassle to merge/wrap things... so we should have the function do it. To control merging, there should be a wrap parameterand anoverwriteparameter. For example: apply_low_pass_filter(tod, wrap='signal_filtered', overwrite=True)` should cause the filtered result to be stored into tod.signal_filtered, even if there is already something stored under that key. Wrapping can be suppressed by passing wrap=None. Whether it is wrapped or not, the function should returned the computed result.
In functions where multiple arrays might be computed and written into the AxisManager... then there are a few possibilities. Perhaps such functions should actually be bundling fields into a sub-AxisManager, which is stored in an input AxisManger under the name specified in the wrap parameter and also returned to the user. It's also reasonable for each array that needs to be stored to be controlled by a separate wrap_ parameter.
The wrap* and overwrite parameters should appear last in the function signature.
TOD Schema
The basic TOD schema, by which we mean the standardization of certain field names in AxisManager, is evolving, ad hoc, directed to some extent by the TOD format documentation but also based on some not-too-carefully thought out SO sim and ACT test data Contexts... The sotodlib standard TOD schema should be formalized a little more carefully and described clearly in the documentation.
Docstrings
We should have a standard format for the data override parameters. We want to communicate where the default will be taken from, and what the basic shape of that array should be. There are probably some cute Sphinx ways to annotate data types and defaults... but here's an example for someone to improve upon:
tod: AxisManager with dets and samps axes.
signal: input data for the filter. Must have shape (tod.dets,
tod.samps). If None, tod.signal is used.
timeconst: time constants to deconvolve, in seconds. Must
have shape (tod.dets). If None, tod.timeconst is used.
Example
At the time of this writing the coords module is not docstrung properly... let's rewrite the signature and docstring for this function: https://github.com/simonsobs/sotodlib/blob/564ee75dacadbf51a8980527d66c5487221bc9d7/sotodlib/coords/helpers.py#L20-L21
Should probably be more like this:
def get_radec(tod, spin=2, focal_plane=None, timestamps=None,
az=None, el=None, roll=None, wrap=False, overwrite=False):
"""Computes (RA, dec, gamma) of each detector from provided
horizon coordinates, timestamps, and focal plane description.
Parameters:
tod: AxisManager with dets and samps axes.
spin: The integer spin weight to apply to the parallactic angle
computation; see below. Only 0, 1 and 2 (default) are valid.
focal_plane: AxisManager describing a standard focal plane (see docs).
Defaults to tod.focal_plane. If this parameter is provided, the 'dets'
axis need not concord with tod, but the result can not be merged into
the tod.
timestamps: Standard timestamps (see docs) to use for pointing.
Defaults to tod.timestamps.
az: Boresight azimuth, in radians. Shape (tod.samps). Defaults to
tod.boresight.az.
el: Boresight elevation, in radians. Shape (tod.samps). Defaults to
tod.boresight.el.
roll: Boresight parallactic rotation, in radians. Shape (tod.samps).
Defaults to tod.boresight.roll.
wrap: If a string, the returned data will also be stored under that name in
tod. If that key already exists, an exception will be raised unless
overwrite=True. Note that if the dets axis specified through the
focal_plane differs from tod.dets, then the result cannot be wrapped and
an error will be raised.
overwrite: Boolean, see wrap argument.
Returns an array with shape (focal_plane.dets, len(timestamps), 4). The 4
elements for each detector-sample are (ra, dec, cos(N*gamma), sin(N*gamma)),
with ra and dec in radians. Note that N=2 is the default, but N=1 may also
be useful. Some modest computational time may be saved by passing N=0.
"""
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by reading the proposed interface and the example entry point in sotodlib/coords/helpers.py, especially get_radec. Review how current TOD-processing functions handle AxisManager inputs, data overrides, wrapping, and docstrings. Done would require an agreed interface, a formalized TOD schema, and an updated example, but the issue does not define which proposal to adopt.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- data-engineering
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100