ComputedFieldsTransform: compute record fields from algebraic expressions, with coefficients and equations supplied from included YAML

Start with ModifyValueTransform and DerivedDataTransform, plus the includes and variable substitution in read_config.py, to see how output-keyed rules differ from the existing input-keyed ones. The design is still open: the cross-instrument section is unfinished, and the safety allowlist needs a maintainer decision first. Done would be a new ComputedFieldsTransform that validates and compiles expressions in init, with tests covering attribute walks, import, lambda, comprehensions and 999.

Updated 2026-10-02 after reviewing how calibration is handled in a
production shipboard deployment. Several custom transforms had been written
there to do what is essentially arithmetic, and looking at them made a good
case for widening this issue's scope: it added the "Cross-instrument
inputs" section below, and expanded the transform from single-record to
optionally stateful. Sections marked [added after later discussion] are new
as of that revision.

Summary

Many data values need calibration applied — usually a linear slope + offset, but not always. Sometimes the correction is a polynomial, and sometimes it needs several fields from the same record (sound velocity from temperature, salinity and depth; a corrected wind speed from speed and heading; a sensor whose response curve is quadratic).

Request: a new ComputedFieldsTransform that computes record fields from algebraic expressions over other fields, where both the coefficients and, where needed, the equation itself can come from an included YAML file rather than being hard-coded in the cruise definition.

This supersedes the linear-only capability of ModifyValueTransform and implements the mapping_function stub in its docstring, whose comment reads "God knows how we'd implement this, but...". ModifyValueTransform should stay exactly as it is, documented as superseded — see below.

Why a new transform rather than extending ModifyValueTransform

ModifyValueTransform keys its fields: dict by input field name and iterates the record:

for field in record.fields:
    field_spec = self.fields.get(field)

That is one-input-per-rule by construction. It is what makes delete_unmatched work, and it is also why a multi-field equation cannot be expressed: there is no single input field to key the rule on.

Fixing that means keying rules by output instead, which changes the meaning of the fields: dict and therefore of every existing cruise config that uses it. Rather than break those, this lands as a sibling transform and ModifyValueTransform is left untouched.

Keying by output is the change that makes everything else fall out — single-input and multi-input rules become the same shape, there is no need for a second config section, and the undefinable question of whether an equation's input counts as "matched" simply disappears.

What this is for, concretely [added after later discussion]

Sites applying calibrations today end up writing a Python transform per
correction, with a matching logger template for each. The kinds of correction
that recur are all arithmetic:

• Linear calibration — a raw count or voltage scaled by slope and offset,
  the common case already served by ModifyValueTransform.
• Polynomial response — a sensor whose calibration curve is quadratic or
  higher.
• Angular correction — an offset applied to a wind direction and wrapped
  back into range.
• Sign from a companion field — a latitude or longitude made signed
  according to a hemisphere field valued 'N'/'S'.
• Conditional terms — a depth derived from pressure, subtracting
  atmospheric pressure only when the sensor reports absolute pressure.
• Multi-field oceanographic formulae — dissolved oxygen corrected for
  temperature, salinity and pressure; sound velocity from temperature,
  salinity and depth; pH compensated for temperature.

Each of these is a few lines of arithmetic wrapped in a module, a class, a
config schema and a template. Collapsing them into one template parameterised
by an equation is what makes this a config-simplification exercise rather than
just another transform.

Proposed interface

class: ComputedFieldsTransform
kwargs:
  # Record-level policy. Both default false - the non-destructive no-op.
  delete_input_fields: false    # fields the equations referenced
  delete_other_fields: false    # fields no equation referenced

  fields:                       # keyed by OUTPUT field name, uniformly
    SeawaterTempCorrected:
      equation: "sbe38_slope * SeawaterTemp + sbe38_offset"
      constants:
        sbe38_slope:  <<sbe38_slope|1.0>>
        sbe38_offset: <<sbe38_offset|0.0>>
      metadata: SBE38 with linear calibration applied

    PressureCorrected:
      equation: "1.0e-5 * Pressure**2 + 0.998 * Pressure - 0.12"

    SoundVelocity:
      equation: <<sound_velocity_eqn>>
      inputs:                   # optional aliasing, so a standard formula can be
        T: SeawaterTemp         # written in textbook notation and reused across
        S: Salinity             # differently-named instruments
        D: Depth
      delete_input_fields: true # optional per-rule override of the global policy

Single-input and multi-input rules are the same shape. The transform iterates its own rules rather than the record's fields, so a rule whose inputs are absent from a record simply does not fire — no special-casing needed for sparse records.

The two flags

Both default false, matching the existing house convention (delete_source_fields, delete_unconverted_fields, delete_unmatched are all delete_*=False; there are no keep_* flags in the codebase). The safe default is also the zero-value default.

They are orthogonal, and all four combinations are meaningful — which is why this is two booleans rather than a mode enum:

delete_input_fields	delete_other_fields	Result	Use case
false	false	record + computed values	inline, enhancing the main data flow — default
true	false	calibrate in place, rest untouched	raw value is meaningless once calibrated
true	true	computed values only	read from CDS, compute, write back
false	true	computed values + their inputs	CDS write-back keeping inputs for context

A mode enum (enhance / replace / derived_only) would name three of those and hide the fourth, and an enum value like replace does not say what it replaces. Two booleans each read clearly in isolation.

delete_input_fields is also settable per rule, where it defaults to the global setting rather than to false — so the global still means something for rules that do not mention it. That covers "consume the inputs to this calibration but leave the inputs to that derived value alone," which one global flag cannot express.

Ordering: deletion must happen after every output has been computed, not as each rule runs, or a field feeding two rules would vanish before the second one reads it.

Middle ground: if someone wants computed values plus a couple of identifying fields, that should be composition rather than a third mode — SelectFieldsTransform(keep=[...]) downstream already does it, consistent with the project's preference for snapping components together. Worth a line in the docstring so nobody proposes keep_fields: as a fifth option later.

The input set comes from the parser for free

Since the expression is parsed with ast anyway for the safety allowlist (below), the Name nodes in an expression are the input field set. So:

• delete_input_fields has a precise, automatic definition — nothing for the user to declare and nothing to guess
• inputs: aliasing is pure optional sugar for readability, not load-bearing
• startup validation can check that every referenced name is either a declared constant or a plausible field name, and report unknowns before the ship sails

This only works if rules are keyed by output, which is another argument for doing it that way.

Calibration files need no new machinery

read_config.py already supports includes: (with wildcard expansion and includes_base_dir), a top-level variables: dict, and substitute_variables() with <<var>>, <<var|default>> and nested defaults. So:

# cals/sbe38.yaml
variables:
  sbe38_slope:  1.00021
  sbe38_offset: -0.0032
  sound_velocity_eqn: "1449.2 + 4.6*T - 0.055*T**2 + 1.39*(S-35) + 0.016*D"

# cruise definition
includes:
  - cals/*.yaml

One consequence worth being explicit about: substitution happens at config-load time, before the transform is constructed. The transform never sees <<...>>, only the resolved text. That is exactly what makes "bring the equation in from a file" work for free — an equation supplied as a variable is just a string that was substituted before __init__ ran. No new config plumbing is needed; this issue is only about how the transform consumes the result.

Safety

This needs to be right: there is currently no eval() or exec() anywhere in logger/ or server/, so this establishes a precedent rather than following one.

Recommended mechanism: parse with ast.parse(expr, mode='eval'), walk the tree, reject any node type not on an explicit allowlist, then compile() and evaluate with {'__builtins__': {}} as globals and only the record fields, declared constants, and a small whitelist of math functions as locals.

Allow: Expression, BinOp, UnaryOp, Constant, Name (load context only), Compare, BoolOp, IfExp, the arithmetic and comparison operator nodes, and Call only to whitelisted names.

Exclusions that carry the weight:

• Attribute — the critical one. Attribute access is the standard sandbox escape: (1).__class__.__mro__[1].__subclasses__() walks from a literal to arbitrary classes and from there to os. An allowlist that permits Attribute is not a sandbox.
• Subscript, Lambda, comprehensions, Starred, walrus, f-strings — no legitimate use in a calibration expression, each worth closing.
• Emptying __builtins__, or __import__('os').system(...) is one call away. Necessary but not sufficient on its own, which is why the node allowlist carries the weight.

A denial-of-service case the AST allowlist does not catch, worth naming explicitly: 9**9**9 is a perfectly ordinary BinOp that will hang the logger and exhaust memory. Mitigate by rejecting ** with large constant exponents, capping expression length and node count, or both.

Validate and compile in __init__, not per record. These transforms run per record at high rate, so re-parsing each time is waste — but more importantly a malformed or rejected expression should fail when the logger starts, not silently mid-cruise. This also lets validate_config catch a bad equation before anyone sails.

Tests should cover the escape attempts, not just the happy path: attribute-walk, __import__, __builtins__ access, lambda, comprehension, and the 9**9**9 exhaustion case.

Dependencies

No safe-expression library is currently installed, and core dependencies were deliberately cut 14 → 8 in August 2026 to keep OpenRVDAS installable on a Raspberry Pi. simpleeval/asteval should therefore not become core dependencies. A stdlib ast evaluator is preferred; if a library is used it belongs in an optional extra, with the transform raising an ImportError naming the install command, as GeofenceTransform does.

Implementation notes

• Subclass DerivedDataTransform, alongside InterpolationTransform, SubsampleTransform and TrueWindsTransform — that is the established base class for transforms that compute new values from existing ones. (The name DerivedDataTransform itself is taken by that base class, hence ComputedFieldsTransform.)
• Accept DASRecord or dict, returning the same shape, via can_process_record() / digest_record() as the other transforms do.
• Carry over from ModifyValueTransform: per-rule metadata, the metadata_interval mechanism, the warning when an output name collides with an existing field, and quiet.
• output_name is not needed — the rule key is the output name.

Cross-instrument inputs: update_on_fields and max_field_age [added after later discussion]

This issue originally scoped the transform as stateless — a pure function of
one record. That is the right starting point, and it covers in-line calibration
completely. But looking at how calibrations are applied in practice makes a good
case for widening it, because the most common multi-input case does not fit.

Where corrections combine readings from more than one instrument, two further
options turn out to be necessary, and they were present on the majority of the
correction loggers reviewed:

update_on_fields:
- <<oxygen_conc_field>>       # recompute only when a new reading arrives
max_field_age:
  <<ctd_water_temp_field>>: 2 # seconds; older cached values count as missing
  <<ctd_salinity_field>>:   2
  <<ctd_pressure_field>>:   2

update_on_fields — compute only when a named input updates. In a
CachedDataServer round-trip you subscribe to several fields arriving at
different rates from different instruments; without this, a new derived value
is emitted every time any secondary input ticks. Depth correction would
re-emit on every GPS latitude update rather than on each new pressure
reading.

max_field_age — per-field staleness tolerance for cached inputs.
Latitude stays usable for 600s; CTD temperature and salinity for 2s. A value
older than its limit counts as missing.

Both require retaining the last value and timestamp of each input across
records, because the CDS delivers fields piecemeal as they change.

These are not new concepts. TrueWindsTransform already has both, with the
same names and semantics — it is the only transform in core that does, and the
convention in those custom transforms is evidently taken from it. Adopting it
here follows existing practice rather than inventing anything.

The dividing line is principled

The corrections that need state are exactly the cross-instrument ones:
oxygen correction pulls temperature and salinity from a CTD that is not the
oxygen sensor; depth correction pulls latitude from a GPS. Those that do not
are single-instrument — voltage to irradiance, one wind field, latitude and
hemisphere arriving in the same sentence.

So state is required precisely when inputs span instruments, which is also the
case that most justifies a multi-input expression transform at all.

Cost

Less than the precedent suggests. TrueWindsTransform spends roughly 95 of its
348 lines on this machinery, but hand-unrolled — five near-identical caching
blocks and five more in _values_too_old — because its five inputs are fixed
and named at compile time.

Here the input set is dynamic and already derived from the AST, so the same
logic is a loop over a dict rather than unrolled blocks: a {field: (value, timestamp)} cache, about ten lines to update it and set the trigger flag, and
about twelve for the presence-and-age check. Call it ~40 lines of production
code including validation of the two new kwargs.

The real cost is conceptual rather than quantitative: the transform stops being
a pure function of one record, which brings lifecycle questions (what happens
on logger restart, what happens when an input never arrives) and tests that
have to manipulate timestamps.

Possible phasing

If this wants containing, the natural seam matches the data: ship the
single-record transform first — covering par, winddir, latlon and every
in-line calibration case — and add the cache as a follow-up for the
cross-instrument ones. The expression engine, safety allowlist and flag design
are unchanged either way; the cache sits in front of evaluation and does not
disturb them.

The argument against phasing is that the cross-instrument corrections were the
majority of those reviewed, so phase one alone would not let such a site retire
much of what it maintains.

Missing inputs

Records are frequently sparse, and a single stream may carry many record types of which only some contain any given rule's inputs. So a rule that cannot fire is the normal case, not an error.

• A rule whose inputs are all absent does not fire, silently.
• warn_missing_inputs, default false — opt in to a warning when a rule cannot fire. It has to default false, because on a mixed stream the ordinary case would otherwise warn on nearly every record.
• When some inputs are present and others are missing, emit a debug line naming which are missing. That is the genuinely suspicious case: the record looks like it was meant for this rule but is incomplete. It is also what you want when chasing a typo'd field name or an instrument that dropped a value.

Three details to settle:

• warn_missing_inputs must be its own flag rather than folded into the inherited quiet, because the polarity differs: quiet=False means "do warn" for everything else in these transforms, whereas missing inputs should default to silence. quiet should stay the master mute — quiet=True suppresses the warning even when warn_missing_inputs is set.
• The debug line should fire once per distinct (rule, set-of-missing-fields), not once per record. A rule always missing the same field on a 10 Hz stream would otherwise bury the log at exactly the moment someone turns up verbosity to investigate.
• Decide whether "missing" covers present but None as well as absent. It should — both mean "no reading" — but the debug message ought to distinguish them, since a field that is always null points at a different upstream problem than one that never appears at all. See #644.
• With caching (above), "missing" acquires a third state: present but too old. It should be treated as missing for the purpose of firing a rule, and named as such in the debug line, since a stale input and an absent one point at quite different problems.

Worth considering separately: a rule that never fires across an entire run is almost certainly a typo'd field name, and that is caught far more cheaply by a report at shutdown than by per-record warnings.

Further requirements [added after later discussion]

Each of these came from a correction being done in practice, rather than being anticipated:

• No blanket coercion of field values to float. Deriving a signed latitude
  means switching on a hemisphere field valued 'N'/'S'. ModifyValueTransform coerces
  everything to float; this transform must not, or string-valued inputs become
  unusable. The Compare and IfExp nodes already on the safety allowlist
  make raw_lat * (-1 if LatHemi == 'S' else 1) expressible, but only if
  strings survive to evaluation.

• Constants must carry non-numeric values, and a null constant should skip
  rather than compute. Calibration coefficients are routinely declared as
  <<cal_coefficient|null>>, so that an uncalibrated device supplies null; a
  rule whose constant is null should decline to fire rather than produce a
  plausible-looking wrong number. Booleans matter too — whether to subtract
  atmospheric pressure when deriving depth is a config flag, not an equation
  change.

• Name the maths whitelist concretely. Oxygen saturation (Garcia-Gordon)
  needs exp and log; UNESCO depth needs sin; pH needs log and
  temperature terms; the wind-direction wrap needs %. So at minimum:
  exp, log, log10, sqrt, sin, cos, atan2, radians, abs, min, max, round,
  with Mod explicitly among the permitted operator nodes.

• Multi-output from shared inputs is load-bearing, not hypothetical.
  Dissolved-oxygen correction emits corrected concentration and saturation
  from one set of inputs, which is what makes the deletion-ordering rule above
  matter in practice.

Other points to settle

• Runtime errors — division by zero, log of a negative, a field arriving as a string. These must fail per-record without killing the logger, and must not emit a warning per record at 10 Hz. Suggest logging once then suppressing, in the style of the existing quiet handling.

Relationship to ModifyValueTransform

Leave it in place and working. The cost of keeping it is a docstring note; the cost of removing it is every existing cruise config. Its docstring should say that ComputedFieldsTransform is the general case and point there, and the mapping_function stub should be dropped or replaced with that pointer.

Out of scope

• Expressions spanning multiple records, or maintaining state.
• Unit awareness / dimensional analysis. Tempting, much larger.

---

Unrelated to this feature, but noticed while reading the code and now filed separately as #644: two bugs in ModifyValueTransform.transform() — dict records always raise because DASRecord() is called with the dict as its first positional (which is json_str), and a None field value raises an uncaught TypeError because the float coercion catches only ValueError.