Fetch and stream 3-phase power #847
Conversation
github-actions
bot
added
part:docs
daniel-zullo-frequenz
changed the title
daniel-zullo-frequenz
force-pushed
the
feature/stream-power-metrics
branch
from
35567c1 to
16e2030
Compare
github-actions
bot
removed
part:docs
daniel-zullo-frequenz
added
the
cmd:skip-release-notes
I skipped release-notes for that ^ reason |
There was a problem hiding this comment.
Just have concerns about the idea of streaming a 3 phase power for an arbitrary meter, because such a value can't be used in a meaningful way, as active power. It can only be used for calculating the power factor, in which case we might as well calculate it in the SDK and stream just that.
If we wanted to stream 3 phase power for its own sake, we'd want to create a formula generator for it instead, and create formulas for grid.power_3_phase, etc.
I'm also unsure of the idea of streaming resampled power values and using it with unresampled voltage values to calculate power factor. It would be much better to do it with values coming from a microgrid.client.meter_data stream rather than going through the resampler/data sourcing actor.
Also, how does this help calculate power factor if we also don't stream the current values for the arbitrary meter?
| async for active_power_sample in active_power_recv: | ||
| print(active_power_sample) | ||
| ``` | ||
| """ |
There was a problem hiding this comment.
this is an ambiguous/confusing interface because the 3phase power values are actually for an arbitrary meter, but active power is not a common quantity like frequency or voltage, and can be different in different places in the microgrid. So microgrid._active_power() without a component id or a location in the graph, tells us nothing.
If you stream the power factor instead using power measurements from the arbitrary meter, then that would be a common value for the whole microgrid.
There was a problem hiding this comment.
It should probably also be called _power_3_phase if we are going this route.
There was a problem hiding this comment.
For the current use-case we only need the 3-phase power factor. I thought we can compute the 3-phase Power Factor on the application side given that the 3-phase grid current (formula generator) and 3-phase voltage (resampled voltage) are already available in the SDK. So the idea was to create a temporary method microgrid._active_power() ("not exposed to the user") to calculate PF = Active Power / (Voltage . Current) in the app and drop it once Power Factor is available in the SDK
Anyway I think this was just a bad idea given the implications it might cause.
I can try to (1) calculate the Power Factor in the SDK or (2) create the 3-phase power formula generator. The (1) needs to be dropped once the power factor is available in the SDK and (2) could be helpful for another scenarios
There was a problem hiding this comment.
The (3) is just to wait until power factor is available and just drop this PR
There was a problem hiding this comment.
You can't use the grid current for this calculation, unless you're also using the 3-phase grid power. The current ActivePowerStreamer streams the 3-phase power of the first_descendant_component, which would be grid power only if the location has a grid-side main meter, which is not everywhere.
There was a problem hiding this comment.
(1) calculate the Power Factor in the SDK
for this you could just find the first_descendant_component and use raw data from it to do the calculations
or (2) create the 3-phase power formula generator
I guess with this, you'd make a 3-phase power formula for grid power?
I think both options are fine as a temporary solution, which ever is easier for you.
There was a problem hiding this comment.
The grid.current is 3-phase current, see https://github.com/frequenz-floss/frequenz-sdk-python/blob/v1.x.x/src/frequenz/sdk/timeseries/grid.py#L99-L118
I guess with this, you'd make a 3-phase power formula for grid power?
Yes, that's what I have in mind.
I'll give a shot to option (2), that should be the easier one 🤞
There was a problem hiding this comment.
done, implemented option (2)
Signed-off-by: Daniel Zullo <daniel.zullo@frequenz.com>
Fetch AC active power from meter, inverter and EV charger components for phase/line 1,2 and 3 respectively. Signed-off-by: Daniel Zullo <daniel.zullo@frequenz.com>
Signed-off-by: Daniel Zullo <daniel.zullo@frequenz.com>
daniel-zullo-frequenz
force-pushed
the
feature/stream-power-metrics
branch
from
16e2030 to
da99c00
Compare
Signed-off-by: Daniel Zullo <daniel.zullo@frequenz.com>
daniel-zullo-frequenz
force-pushed
the
feature/stream-power-metrics
branch
from
da99c00 to
1962a0d
Compare
src/frequenz/sdk/timeseries/formula_engine/_formula_generators/_grid_power_3_phase_formula.py
Outdated
Show resolved
Hide resolved
daniel-zullo-frequenz
force-pushed
the
feature/stream-power-metrics
branch
from
1962a0d to
2a9f2bc
Compare
|
Renamed |
| if idx > 0: | ||
| formula_builder.push_oper("+") | ||
|
|
||
| # When EV chargers produce `None` samples, they are excluded from | ||
| # the calculation by treating their `None` values as `0`s. | ||
| # | ||
| # This is not possible for Meters, so when they produce `None` | ||
| # values, those values get propagated as the output. | ||
| if comp.category in (ComponentCategory.EV_CHARGER,): | ||
| nones_are_zeros = True | ||
| elif comp.category == ComponentCategory.METER: | ||
| nones_are_zeros = False | ||
| else: | ||
| continue |
There was a problem hiding this comment.
Don't you want to group inverters with ev chargers?
Also I realized there is a bug in this approach: If we encounter an unhandled category like inverter, we will continue. But we've already added a + step, so we're continuing without adding a component after the +.
I think we have to move the if idx > 0: ... stuff after the category check, so that we add a + only if we're going to insert a component.
Might affect other formulas as well.
There was a problem hiding this comment.
Makes sense. Should I apply the same patch for grid current ?
There was a problem hiding this comment.
Don't you want to group inverters with ev chargers?
Good catch! Yes, I do want to group them
There was a problem hiding this comment.
Updated as suggested
| ), | ||
| phase_3=electrical_pb2.AC.ACPhase( | ||
| current=metrics_pb2.Metric(value=34.5), | ||
| voltage=metrics_pb2.Metric(value=230.2), | ||
| power_active=metrics_pb2.Metric(value=3680.1), |
There was a problem hiding this comment.
maybe you want to add an assert for these values as well?
There was a problem hiding this comment.
yes, I do want that. Thanks! Fixed
Signed-off-by: Daniel Zullo <daniel.zullo@frequenz.com>
The 3-phase power is needed to calculate the power factor of the microgrid. The power factor is not available through the microgrid API in the current version used in the SDK. So the 3-phase power is temporary exposed through the grid until the migration to the latest version is completed. Then the 3-phase power factor can be fetched and streamed through the microgrid. Signed-off-by: Daniel Zullo <daniel.zullo@frequenz.com>
Signed-off-by: Daniel Zullo <daniel.zullo@frequenz.com>
daniel-zullo-frequenz
force-pushed
the
feature/stream-power-metrics
branch
from
2a9f2bc to
12f7bf0
Compare
Add the formula operator if and only if there is a component to add. Otherwise, this was adding a `+` operator, but not adding a component after the `+`. Signed-off-by: Daniel Zullo <daniel.zullo@frequenz.com>
Signed-off-by: Daniel Zullo <daniel.zullo@frequenz.com>
daniel-zullo-frequenz
force-pushed
the
feature/stream-power-metrics
branch
from
12f7bf0 to
ca22472
Compare
daniel-zullo-frequenz
removed
the
cmd:skip-release-notes
The 3-phase active power is temporary needed to calculate the power factor of the microgrid. The power factor is not available through the microgrid API in the current version used in the SDK.
So the 3-phase active power is temporary exposed through the microgrid API until the migration to the latest version is
completed. Then the 3-phase power factor can be fetched and streamed through the microgrid API.
Temporary workaround for #841.