Repository navigation
Evaluate composed formulas in the logical-meter actor - #60
Conversation
44dc678 to
5592c53
Compare
4c3d843 to
d1143a4
Compare
The handle tags every component leaf of a graph formula with its metric, so the actor gets one expression type for every metric and evaluates each formula against one snapshot of resampled values per tick. Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>
`Formula` is now a typed wrapper over a formula-engine expression. Composing formulas only builds the expression, so composition cannot fail. The actor evaluates the whole expression against one tick's snapshot, so operands no longer need lining up by timestamp. The per-operator tasks, channels and timestamp synchronisation go away, and so do operands of another quantity and operands that are raw receivers. One logical meter evaluates a whole expression, so an operand from another meter would be resampled by this meter, with its config and possibly through another microgrid's client: wrong data or no data, and no error. Composition cannot fail, so a mixed formula drops its logical meter and `subscribe()` returns an error instead. Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>
Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>
Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>
Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>
Prepares the logical meter for evaluating composed formulas in the actor (frequenz-floss#60, now split into a stack of three PRs; this is the first). It moves to formula engine 0.2 and component graph 0.6.3, picks graph formulas by a crate-private metric kind instead of per-kind provider impls, and replaces the actor's per-quantity maps and downcast response channel with per-subscriber sinks. ## Changes - `Depend on formula engine 0.2`: missing components now read as unknown; NaN and infinity become missing data. - The handle picks the graph method from `(FormulaKind, FormulaTarget)` in one match, replacing `GraphFormulaProvider` and its macros. - The actor keeps one entry per distinct formula with a sink per subscriber, and prunes formulas and resamplers nobody reads on every tick. `subscribe()` no longer waits for the actor. - The handle parses graph formulas, so a formula the engine rejects (e.g. over its operator limit) fails in the handle method with `FormulaEngineError`, instead of failing `subscribe()`. - Requires component graph 0.6.3, whose consumer and producer formulas are no longer clamped at zero. Without a lock file, `^0.6.2` already resolved to it. - Focused tests for sink pruning and resampler dropping, the full kind-by-target dispatch table, and the engine's rejection of an oversized consumer formula. ## Breaking `Metric::FormulaType` and `ErrorKind::DroppedUnusedFormulas` are removed, `Quantity` is sealed, and formula values change for NaN/infinite inputs, oversized graph formulas, and the no-longer-clamped consumer and producer formulas. See `## Upgrading` in RELEASE_NOTES.md.
d1143a4 to
18431b8
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Deep composed expressions bypass the formula engine’s depth safeguard and can overflow the process stack.
1 open finding
What changed in this PR
Continues the formula-composition redesign from #64 by moving composed-formula evaluation into the logical-meter actor.
Changes:
- Introduces typed expression-backed formulas with metric-tagged component keys.
- Evaluates formulas against one resampled snapshot per tick.
- Removes legacy asynchronous composition and documents breaking API changes.
| File | Description |
|---|---|
RELEASE_NOTES.md |
Documents formula API and behavior changes. |
README.md |
Adds a formula-composition example. |
examples/logical_meter.rs |
Updates composition for the infallible API. |
src/lib.rs |
Exports FormulaOperand. |
src/logical_meter.rs |
Updates formula re-exports. |
src/logical_meter/formula.rs |
Implements expression-backed formulas and composition. |
src/logical_meter/formula/async_formula.rs |
Removes legacy asynchronous formulas. |
src/logical_meter/formula/async_formula/application_methods.rs |
Removes client-side evaluation helpers. |
src/logical_meter/formula/async_formula/composition.rs |
Removes legacy composition logic. |
src/logical_meter/formula/graph_formula.rs |
Removes the graph-formula wrapper. |
src/logical_meter/logical_meter_actor.rs |
Evaluates formulas from keyed snapshots. |
src/logical_meter/logical_meter_handle.rs |
Tags graph leaves with metrics. |
src/microgrid/pv_pool.rs |
Updates formula display expectations. |
src/microgrid/steam_boiler_pool.rs |
Updates formula display expectations. |
src/quantity.rs |
Adds base-unit value extraction. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| /// one evaluation in the logical meter. | ||
| /// | ||
| /// Fails if the formula combines formulas from different logical meters. | ||
| pub async fn subscribe(&self) -> Result<broadcast::Receiver<Sample<Q>>, Error> { |
There was a problem hiding this comment.
The async composition this replaces had the same recursion in Display/subscribe/Drop, so this isn't new, but the actor now also walks the tree every tick. A subscribe() check would bound the actor, not Drop/clone on the caller's side. I'll add the depth guard in a follow-up PR.
Also, it doesn't crash at 1024. On debug builds it needs it to be 3000 levels and more than 16000 levels in release builds.
There was a problem hiding this comment.
Actually, was going to fix this in the formula engine, will just do it there.
llucax
left a comment
llucax
left a comment
There was a problem hiding this comment.
Pretty nice! I did a design/high-level review, and delegated the rust part to copilot, but if any other rustacean can have a look it would probably be good.
| "METRIC_AC_POWER_ACTIVE::(COALESCE(#5 + #4, #3, COALESCE(#5, 0.0) + COALESCE(#4, 0.0)))" | ||
| concat!( | ||
| "COALESCE(#5:AC_POWER_ACTIVE + #4:AC_POWER_ACTIVE, #3:AC_POWER_ACTIVE, ", | ||
| "COALESCE(#5:AC_POWER_ACTIVE, 0) + COALESCE(#4:AC_POWER_ACTIVE, 0))" | ||
| ) |
There was a problem hiding this comment.
This looks very neat. Does it mean that now the formula will check metric units/quantities, so you can't do #5:AC_POWER_ACTIVE + #5:AC_CURRENT and the type is inferred so #5:AC_POWER_ACTIVE / #5:AC_CURRENT gives you a Voltage or something like that, or are we not there yet?
There was a problem hiding this comment.
The string formulas don't support this, and might never support them.
It will reject Formula<Power> + Formula<Current>, but will also reject Formula<Power> / Formula<Current>.
Active Power / Current is not voltage, because of power factor. Current * Voltage is Apparent power in AC, so when we have apparent power, we can definitely implement Current * Voltage and related things.
| Formulas compose without subscribing. `+`, `-`, `* f32`, `/ f32`, `* Percentage`, `coalesce`, `min`, `max` and `avg` build one expression. The logical meter evaluates it once per resampling tick, so all operands, of any metric, are read from the same tick's resampled values. | ||
|
|
||
| ```rust , ignore | ||
| use frequenz_microgrid::{metric, quantity::Power}; | ||
|
|
||
| let logical_meter = microgrid.logical_meter(); | ||
| let net = logical_meter.grid::<metric::AcPowerActive>()? | ||
| - logical_meter.pv::<metric::AcPowerActive>(None)? | ||
| + Power::from_kilowatts(100.0); | ||
| let mut rx = net.subscribe().await?; |
There was a problem hiding this comment.
Very nice that you can build your formulas but decide when to actually subscribe to them. This was done somehow with the goal of interfacing better with Python and async, right? So the idea is to eventually do all the formula parts in here but be able to get the actual data asynchronously in Python, or is it a different plan / I'm misunderstanding?
There was a problem hiding this comment.
Yes, the formula engine is actually sync, so we don't need any async until we're ready to subscribe.
We can get composition instructions from python through the API and stream values back through the API as well. So we can have all kinds of flexibility in Python.
Second PR of the formula-composition stack (after #64).
Formula<Q>becomes a typed wrapper over a formula-engine expression whose leaves carry the metric, and the logical-meter actor evaluates every formula against one snapshot per resampling tick. This replaces the per-operator tasks, channels and timestamp synchronisation.Changes
+,-,* f32,/ f32,* Percentage,coalesce,min,max,avg) only builds an expression and cannot fail. The old async composition (~1600 lines) is deleted.subscribe()fails withFormulaEngineErrorinstead of resampling another meter's components with the wrong config.Displayshows each component with its metric (#2:AC_POWER_ACTIVE); the tests pin exact strings for every graph target.src/logical_meter/formula.rs(new) and the actor'sresample/evaluate_formulas. The actor's subscription bookkeeping (handle_subscribe_formula,drop_unused_resamplers) is replaced in the next PR by on-demand subscriptions.Breaking
Formula<Q>is a struct,FormulaSubscriberand the enum variants are gone, composition no longer returnsResult,FormulaOperandis public, receivers can no longer be operands, mixed-meter formulas fail atsubscribe(), andDisplaychanged. See## Upgradingin RELEASE_NOTES.md.