-
Notifications
You must be signed in to change notification settings - Fork 20
Switch to the rust component graph #1295
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
base: v1.x.x
Are you sure you want to change the base?
The head ref may contain hidden characters: "new-formulas+new-graph=\u{1F389}"
Conversation
This introduces the new microgrid API v0.18. We also need to bump the frequenz-client-common dependency to at least v0.3.6, so it can work with the new microgrid client. Signed-off-by: Leandro Lucarella <[email protected]>
The microgrid API now doesn't support not reporting a rated fuse for a grid connection, so we don't need to test for that case anymore. Signed-off-by: Leandro Lucarella <[email protected]>
The battery pool bounds calculation is buggy and these tests are wrong (see frequenz-floss#1180). By switching to using ranges of bounds, the buggy behaviour changes and make these tests fail. Fixing them is difficult without switching to using ranges natively first, so we just skip these tests for now. Signed-off-by: Leandro Lucarella <[email protected]>
This will help us to make sure all sub-classes are properly updated when doing changes to method signatures. Signed-off-by: Leandro Lucarella <[email protected]>
The new API doesn't return component data packets with one timestamp and several metrics anymore, so we write an adaptation layer to convert individual metrics to `ComponentData` wrappers so we don't need to update every location where streaming data is being consumed. Signed-off-by: Leandro Lucarella <[email protected]>
This change updates the SDK to use the new `Metric` enum from `frequenz.client.microgrid.metrics` (and the new `TransitionalMetric`) in place of the old `ComponentMetricId`. It adapts internal types and mappings to accept `Metric | TransitionalMetric` where appropriate. Imports, request/channel naming, and data-extraction maps across the microgrid data sourcing and timeseries modules (battery pool, voltage/ frequency streamers, formula engine, etc.) have been updated to the new metric names (for example `ACTIVE_POWER` → `AC_ACTIVE_POWER`, etc.), and a `TransitionalMetric` shim is used to preserve compatibility for metrics that are still referenced by older code paths. Tests and benchmarks were updated accordingly. Signed-off-by: Leandro Lucarella <[email protected]>
For non data-streaming methods, update to use the new client method names. For data-streaming methods, use the new adaptation functions in `_old_component_data.py`. Because `set_component_power_active()` can now return a the time when the command expires, we also need to update the tests we use to run them. Note that the data-streaming method in the new client version is not async (it returns a Receiver synchronously) so `await`s are also removed. For the `ComponentMetricFetcher` we also remove the `async` for the wrapper `_subscribe()` methods. The mock microgrid client was mostly rewritten to adjust it to the new unified data streaming method. Signed-off-by: Leandro Lucarella <[email protected]>
API is too generic, a more specific name makes the code more clear. Signed-off-by: Leandro Lucarella <[email protected]>
The new `Component` class renamed `component_id` with `id`, so we need to update all our references. Signed-off-by: Leandro Lucarella <[email protected]>
The name in the microgrid API will be changed again to `Microgrid` in the near future, so internally we just use `microgrid` instead of `microgrid_info`. Signed-off-by: Leandro Lucarella <[email protected]>
The new API client provides a mapping of `ComponentCategory` and `XxxType`s to Python classes. This commit updates most uses of component categories with these classes. This also includes some updates of imports for `Component` and `ComponentCategory` and some changes on specific component type, like `GridConnectionPoint`'s metadata. Finally, the component graph interface to get components was also updated to use types instead of categories, for which the API was slightly changed to better represent the filtering criteria. The keyword for arguments were changed from `component_ids` to `matching_ids` and `component_categories` to `matching_types`. Signed-off-by: Leandro Lucarella <[email protected]>
While we are changing the interface of component retrieval in the component graph, we take the opportunity to make its interface more generic. We now accept any Iterable for `matching_ids` and `matching_types`, as well as one individual items. Most uses of it filter by a single item anyway so it is annoying to have to wrap it in a Iterable. Most uses of those methods were updated to use the single-item overload. Signed-off-by: Leandro Lucarella <[email protected]>
The new API client can return also an `int` as a component category when the client doesn't know the category number. Signed-off-by: Leandro Lucarella <[email protected]>
To match the new component graph filtering arguments for `components()`, the `connections()` method now also have its argument names updated: * `start` -> `matching_sources` * `end` -> `matching_destinations` Signed-off-by: Leandro Lucarella <[email protected]>
Like with component graph `components()`, now we support passing a single element to `matching_sources` and `matching_destinations`. The `components()` function is also simplified to make filtering more compact. Also updates uses with a single element to remove the unnecessary container. Signed-off-by: Leandro Lucarella <[email protected]>
Many tests build mock components, so now this components need to be created as concrete subclasses of `Component`. Some test also has some minor readability improvements. Signed-off-by: Leandro Lucarella <[email protected]>
The new state snapshots in the v0.18 client provide errors and warnings using a different structure, so we need to account for that. State code were also unified and there is no more specific enums for cable state and other component-specific states, all states are part of the `ComponentStateCode` enum and the component data can have many state codes in it, while before it contained only one. In previous versions EV chargers only used one `PLUGGED` or `LOCKED` status but now the cable status was split in 2 connector ends: station and EV, so now all combinations of those need to be accounted for. This might change again on the API side as it ended up a bit messy. Also the old component data wrappers now can't be frozen because the `ComponentData` class is not frozen (this is to allow parsing from the new telemetry messages easier for phases). Signed-off-by: Leandro Lucarella <[email protected]>
Signed-off-by: Leandro Lucarella <[email protected]>
Signed-off-by: Sahas Subramanian <[email protected]>
Signed-off-by: Sahas Subramanian <[email protected]>
Signed-off-by: Sahas Subramanian <[email protected]>
It sends requests to the resampler and fetches resampled telemetry streams. Signed-off-by: Sahas Subramanian <[email protected]>
Waits for new values from the input data streams. When there's one new value from each of them, evaluates the AST and sends the calculated value out. Signed-off-by: Sahas Subramanian <[email protected]>
This is a wrapper around the FormulaEvaluatingActor, with methods for composing multiple formulas. Signed-off-by: Sahas Subramanian <[email protected]>
Signed-off-by: Sahas Subramanian <[email protected]>
Signed-off-by: Sahas Subramanian <[email protected]>
This also adds an evaluator and tests for the 3-phase formulas. Signed-off-by: Sahas Subramanian <[email protected]>
Signed-off-by: Sahas Subramanian <[email protected]>
This will replace the SDK's old component graph and the formula generators. This commit also adds some graph traversal methods that are not provided by the component graph library yet. Signed-off-by: Sahas Subramanian <[email protected]>
Graphs in island mode are not supported yet. Earlier, this test was using an `Unspecified` component category as a junction node, to start traversing from, which is inaccurate. The `Unspecified` component should always be an error, to identify an unfilled category field, due to a bug. Signed-off-by: Sahas Subramanian <[email protected]>
Signed-off-by: Sahas Subramanian <[email protected]>
Signed-off-by: Sahas Subramanian <[email protected]>
Signed-off-by: Sahas Subramanian <[email protected]>
Signed-off-by: Sahas Subramanian <[email protected]>
This won't be necessary as soon as the coalesce AST node is able to subscribe to secondary streams only if primary streams are not available. But for now, coalesce nodes need all their input streams to have data. Signed-off-by: Sahas Subramanian <[email protected]>
The new formulas use component metrics as primary and only when those are not available, uses any corresponding meters as fallback. Signed-off-by: Sahas Subramanian <[email protected]>
Signed-off-by: Sahas Subramanian <[email protected]>
Signed-off-by: Sahas Subramanian <[email protected]>
|
Based on #1283 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR switches from a Python-based component graph and formula engine to a new Rust-based implementation, introducing new Formula and Formula3Phase classes to support advanced formula functions (max/min/coalesce) and removing deprecated code.
Key Changes:
- Replaced old Python formula engine with new Rust-based component graph
- Implemented new
FormulaandFormula3Phaseclasses with support for max/min/coalesce functions - Updated component data structures to use new client library types
- Removed deprecated formula engine tests and implementation
Reviewed Changes
Copilot reviewed 109 out of 118 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/timeseries/test_formula_engine.py | Entire file deleted - old formula engine tests removed |
| tests/timeseries/test_consumer.py | Added missing resampler calls for battery and PV inverter power |
| tests/timeseries/mock_resampler.py | Updated to use new Metric enum instead of ComponentMetricId |
| tests/timeseries/mock_microgrid.py | Updated component types and connection structures to match new API |
| tests/timeseries/_formulas/test_lexer.py | New test file for formula lexer functionality |
| tests/timeseries/_formulas/test_formulas_3_phase.py | New test file for three-phase formula support |
| tests/timeseries/_formulas/test_formulas.py | New comprehensive formula tests with max/min/coalesce support |
| tests/timeseries/_formulas/test_formula_composition.py | Updated to use new formula API and async context managers |
| src/frequenz/sdk/timeseries/formulas/_token.py | New token definitions for formula parsing |
| src/frequenz/sdk/timeseries/formulas/_resampled_stream_fetcher.py | New component for fetching resampled telemetry streams |
| src/frequenz/sdk/timeseries/formulas/_peekable.py | New peekable iterator implementation for parsing |
| src/frequenz/sdk/timeseries/formulas/_parser.py | New formula parser implementation |
| src/frequenz/sdk/timeseries/formulas/_lexer.py | New formula lexer implementation |
| src/frequenz/sdk/timeseries/formulas/_functions.py | New function implementations (max/min/coalesce) |
Comments suppressed due to low confidence (1)
tests/timeseries/mock_microgrid.py:9
- The import
Callablefromcollections.abcon line 10 is unused after the changes. TheCallabletype is imported again on line 12 from thetypingmodule via theCoroutineimport, and all usages in the file appear to use the typing module'sCallable(e.g., line 260). This unused import should be removed.
from collections.abc import Callable
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tests/microgrid/power_distributing/_component_status/test_battery_status.py
Show resolved
Hide resolved
Signed-off-by: Sahas Subramanian <[email protected]>
4583c7d to
c942e5f
Compare
|
Can you rebase it on top of Based on #1293 instead? |
Formula/Formula3Phaseclasses to replace the old formula engine, for supporting the new formula functions - max/min/coalesce.