frequenz-floss / frequenz-sdk-python

Frequenz Python Software Development Kit (SDK)
https://frequenz-floss.github.io/frequenz-sdk-python/
MIT License
13 stars 17 forks source link

Support component graph configs with m:n battery-inverter connections #501

Open shsms opened 1 year ago

shsms commented 1 year ago

What's needed?

Currently multiple parts of the SDK expect that each battery is connected to a single inverter. This needs to be generalized, such that there can be configs like

Proposed solution

This needs to be implemented and/or tested in these modules:

Notes

Taken from the discussion below: We can always assume that all inverters are equal, e.g. it doesn't matter in which part of the microgrid they release power. The currently discussed and preferred solution is to stack inverters to max. efficiency (lower power -> lower efficiency)

Marenz commented 1 year ago

How do we want to deal with multiple inverters connected to one (or multiple) batteries? What inverters should a charge/discharge command affect? @thomas-nicolai-frequenz @tiyash-basu-frequenz

shsms commented 1 year ago

Just for context, when the battery pool is used to set power, we only sent the battery ids to the power manager/distributor. And they have to find the corresponding inverter. Do we just distribute power equally if there are multiple inverters for a single battery?

thomas-nicolai-frequenz commented 1 year ago

Do we just distribute power equally if there are multiple inverters for a single battery?

As a default I'd say yes but we should support multiple strategies in the future as the most efficient way is to run inverters close to their max instead of within lower power ranges. This is when they become less efficient by default. That means the best case would be to enable just one and start enabling more if the power increases beyond a single inverter. However, keep in mind that we might also need to look into failover. Once inverter might fail but the second one still work. Thats something we do not have in a 1:1 setup.

@tiyash-basu-frequenz any thoughts on that?

thomas-nicolai-frequenz commented 1 year ago

one inverter to multiple batteries

is a different case.

Marenz commented 1 year ago

one inverter to multiple batteries

is a different case.

but that's exactly the case we're currently want to discuss here?

thomas-nicolai-frequenz commented 1 year ago

but that's exactly the case we're currently want to discuss here?

its one of them but the least challenging one imho. There is no power to be distributed in that case as there are multiple batteries on the DC bus and you can't distribute power dynamically. So there is nothing to be done from a power distributor perspective.

Marenz commented 1 year ago

Does that mean we always assume (or don't want to support other configurations) that multiple inverters connected to one (or more) battery always connect to the same part of the microgrid so that it technically doesn't matter which inverter we use and we only want to use more if a single inverter can't handle all the available power?

tiyash-basu-frequenz commented 1 year ago

the most efficient way is to run inverters close to their max instead of within lower power ranges. This is when they become less efficient by default. That means the best case would be to enable just one and start enabling more if the power increases beyond a single inverter.

That's a very good point. It makes sense to stack inverters, instead of distributing equally.

Once inverter might fail but the second one still work.

Right. The PowerDistributor should be able to exclude inverters that are off or in an error state (IIUC a logic for this already exists for this somewhere in the SDK).

tiyash-basu-frequenz commented 1 year ago

@Marenz When there are multiple batteries connected to an inverter on its DC bus, then it is the inverter that controls how much power gets assigned to each battery. This is not something that the SDK can influence.

In general, we never ask a battery to charge or discharge. We always ask the inverter, and the inverter takes care of pulling/pushing power from/to the battery. The same holds true if there are multiple batteries attached to an inverter.

thomas-nicolai-frequenz commented 1 year ago

Does that mean we always assume (or don't want to support other configurations) that multiple inverters connected to one (or more) battery always connect to the same part of the microgrid so that it technically doesn't matter which inverter we use and we only want to use more if a single inverter can't handle all the available power?

We have two main set ups we need to support right now:

1 inverter => m batteries
n inverters => 1 battery
thomas-nicolai-frequenz commented 1 year ago

multiple batteries connected to multiple inverters

This case I haven't see yet. @tiyash-basu-frequenz do you?

tiyash-basu-frequenz commented 1 year ago

multiple batteries connected to multiple inverters

This case I haven't see yet. @tiyash-basu-frequenz do you?

We have never encountered multiple inverters connected the same DC bus in general.

Marenz commented 1 year ago

This issue specifically wants n:m support as well though..

multiple batteries connected to multiple inverters

@Marenz When there are multiple batteries connected to an inverter on its DC bus, then it is the inverter that controls how much power gets assigned to each battery. This is not something that the SDK can influence.

Sure, but that was never something I asked or questioned..

In general, we never ask a battery to charge or discharge. We always ask the inverter, and the inverter takes care of pulling/pushing power from/to the battery. The same holds true if there are multiple batteries attached to an inverter.

Again, my question was more aimed at the other side (AC) of the inverters. Do we assume all AC exit points of multiple inverters connected to the same batteries connect to the same section of the microgrid/grid

Batt1; Batt2 -> Inverter1, Inverter2

Inverter1 -> microgrid-part-a Inverter2 -> microgrid-part-b

is it assumed that part a and b are connected?

shsms commented 1 year ago

I had mentioned m:n combinations based on an earlier conversation, but we can probably cover later if necessary.

shsms commented 1 year ago

Do we assume all AC exit points of multiple inverters connected to the same batteries connect to the same section of the microgrid/grid

I think so, we won't connect them in such a way that it will make a difference. A difference would only be possible if there are two microgrids. So I think we can always assume that's the case.

tiyash-basu-frequenz commented 1 year ago

Again, my question was more aimed at the other side (AC) of the inverters. Do we assume all AC exit points of multiple inverters connected to the same batteries connect to the same section of the microgrid/grid

Sorry for misreading your question.

But no, we cannot assume that. In theory, two inverters could have the same battery/DC bus, but link to different parents. But maybe we want to limit the use-case now to all inverters connecting to the same node on the AC side.

@thomas-nicolai-frequenz I think this raises a few points about the microgrid graph in general, and if we want to constraint it. Maybe we will have more visibility once we encounter or research about such setups.

thomas-nicolai-frequenz commented 1 year ago

A difference would only be possible if there are two microgrids.

If the use case that there are two batteries in a microgrid and each having one inverter and each inverter has a different parent thats already the case as mostly the parent is a distinct meter. I'm not really sure can follow your thoughts.

Even in the case of multiple inverters per one battery each inverter might have its own meter in front of it and hence would be already connected to a different "part" as in parent component.

Marenz commented 1 year ago

I am not sure the "parent" term is helpful here. Inverters with different parents can still connect to the same part of the grid if they eventually have a (non-inverter-interrupted) connection to common parts eventually

thomas-nicolai-frequenz commented 1 year ago

I am not sure the "parent" term is helpful here. Inverters with different parents can still connect to the same part of the grid if they eventually have the a (non-inverter-interrupted) connection to common parts eventually

Can we define what "part of the microgrid" means? What is a common part? And why is this related to this issue here?

shsms commented 1 year ago

If the use case that there are two batteries in a microgrid and each having one inverter and each inverter has a different parent thats already the case as mostly the parent is a distinct meter. I'm not really sure can follow your thoughts.

I'm just saying that being connect to different meters won't make a difference, because they will still have a single virtual AC junction point, on which Kirchoff's laws would apply, so choosing one inverter over the other in m inv -> 1 bat setups wouldn't make a difference. Unless an inverter or meter is broken, we can just pick any inverter first, and they would all operate on the single kirchoff laws point.

Marenz commented 1 year ago

Let me rephrase the question like this: Do we want to support configurations where it makes a difference which inverter is used when multiple are connected to the same battery(or set of batteries).

This is related because when there are multiple inverters for a battery, I need to know if I can use all of them or not for power requests..

thomas-nicolai-frequenz commented 1 year ago

You can always use all inverters that are connected to a battery. I'm not aware of any restrictions beyond fuse limits or performance considerations (stacked-usage as pointed out by @tiyash-basu-frequenz above). At the very end the inverters are all connected to the same microgrid (AC bus) as @shsms pointed out. That means "where" the power gets physically released within the microgrid doesn't matter and in doubt we don't even know it because how the electrician set up the wiring is usually not (and can not be) 1:1 mirrored in the component graph. Thats almost impossible to be done and again it also doesn't matter. Its not like electricians provide a perfect wiring plan. Sometimes even they don't know where cables are running along etc. All it matters is that its behind the metering point(s) that belong to the microgrid.

thomas-nicolai-frequenz commented 1 year ago

One important note: If an inverter connects to a different metering point belonging to a different microgrid we made a mistake in the setup. Hence there is always only ONE microgrid but it can have no, one or multiple grid connections, multiple for redundancy reasons for example.

shsms commented 1 year ago

the other thing I was concerned about is the case of n-batteries -> 1 inverter. If we receive a request in the power distributor, I feel like we should accept requests only for all the n batteries together. Maybe we shouldn't even allow those n batteries to be separated when creating a battery pool instances.

Because otherwise, there will be weird overlaps, and the implementation will get super complicated, because we'll have to remember the power requests made for each of the batteries separately and sum that before sending to the single inverter.

And then we'd have to track the age of older requests for other batteries, etc, because we are storing them. I feel like this is all complications we should avoid in the sdk.

Marenz commented 1 year ago

It seems our graph nx library nx.is_tree() doesn't accept graphs with two inverters connected to the same battery. The example they give themselves is:

    >>> G = nx.Graph()
    >>> G.add_edges_from([(1, 2), (1, 3), (2, 4), (2, 5)])
    >>> nx.is_tree(G)  # n-1 edges
    True
    >>> G.add_edge(3, 4)
    >>> nx.is_tree(G)  # n edges
    False
tiyash-basu-frequenz commented 1 year ago

If we receive a request in the power distributor, I feel like we should accept requests only for all the n batteries together. Maybe we shouldn't even allow those n batteries to be separated when creating a battery pool instances.

If all the n batteries are on the same DC bus, then we won't be in control of individual batteries anymore. Makes sense to ask users to intentionally mention all batteries on a single bus.

Because otherwise, there will be weird overlaps, and the implementation will get super complicated, because we'll have to remember the power requests made for each of the batteries separately and sum that before sending to the single inverter. And then we'd have to track the age of older requests for other batteries, etc, because we are storing them. I feel like this is all complications we should avoid in the sdk.

Agree. Besides the case of all batteries being on the same DC bus, some inverters may have separate DC buses for individual batteries. Supporting it will require changes on the backend side too. So, not something we should aim for now.

llucax commented 1 year ago

So maybe the battery pool could be sort of best-effort, like you request for batteries 1,2,3, but it could get batteries 1,2,3,4,5 in the pool because batteries 2,4 and batteries 3,5 share an inverted/DC bus?

There could even be a parameter to select if you prefer to increase or decrease capacity in case the request can't be fulfilled exactly?

llucax commented 1 year ago

It seems our graph nx library nx.is_tree() doesn't accept graphs with two inverters connected to the same battery. The example they give themselves is:

    >>> G = nx.Graph()
    >>> G.add_edges_from([(1, 2), (1, 3), (2, 4), (2, 5)])
    >>> nx.is_tree(G)  # n-1 edges
    True
    >>> G.add_edge(3, 4)
    >>> nx.is_tree(G)  # n edges
    False

Well, nx is right, that's not a tree, that's a DAG. We already discussed this somewhere. I guess we need to get rid of the tree check. Not sure if nx has a DAG check, because we definitely don't want to have cycles, right?

llucax commented 1 year ago

https://networkx.org/documentation/stable/reference/algorithms/dag.html

shsms commented 1 year ago

So maybe the battery pool could be sort of best-effort, like you request for batteries 1,2,3, but it could get batteries 1,2,3,4,5 in the pool because batteries 2,4 and batteries 3,5 share an inverted/DC bus?

That would be very confusing if someone has a setup where they want to use one battery for one use and another battery for a different use, and they create two battery pools, but if battery pool implicitly converges into the same pool, that would be very confusing. If anything is specified, I think we should should use just the specified batteries, and if that's not possible, raise an error instead.

thomas-nicolai-frequenz commented 1 year ago

So maybe the battery pool could be sort of best-effort, like you request for batteries 1,2,3, but it could get batteries 1,2,3,4,5 in the pool because batteries 2,4 and batteries 3,5 share an inverted/DC bus?

@llucax I'm not sure this can happen, should happen and/or is even correct. If two batteries share the same inverter they are in fact ONE logical unit and hence just a single battery in a BatteryPool. For Example, there is no way you can discharge battery 2 but not 4 if both are connected to the same inverter/dc-bus.

thomas-nicolai-frequenz commented 1 year ago

Maybe we shouldn't even allow those n batteries to be separated when creating a battery pool instances.

@sahas-subramanian-frequenz correct!

I feel like this is all complications we should avoid in the sdk.

Yes! It does't even make sense. If two batteries share the same inverter there is no way to individually address a battery. (Dis)charging will always be for both. Hence, the separation into two pools wouldn't achieve much.

llucax commented 1 year ago

It is clear that if 2 batteries share the same inverter, then they have to be handled in a group. The issue was more about what to do if battery 1 and 2 share an inverter and the user asks to create a battery pool with batteries 1,3.

Then we can either include 2 too under the hood (so you'll end up with a battery pool with batteries 1,2,3) or to raise an error. I agree it is less confusing to raise an error, but this means the developer will have to know about the topology in a bit more detail than just knowing how many batteries are there, they need to know where inverters are otherwise they will get an error (maybe we can make it easy from the error to be able to re-request the battery pool with more or less batteries, but still that will be guessing and you could end up with 2 battery pools that are effectively the same).

This was my main rationale to suggest an adaptative approach. If we can assume users (and apps) will know the exact topology when creating battery pools, then I agree we don't need to go for that.

tiyash-basu-frequenz commented 1 year ago

If we can assume users (and apps) will know the exact topology when creating battery pools, then I agree we don't need to go for that.

Maybe we can assume that if a user requests for specific battery IDs, they know the topology?

We could also add a util method that returns a list of all the "sibling" batteries to a given input battery ID.

thomas-nicolai-frequenz commented 1 year ago

I agree it is less confusing to raise an error, but this means the developer will have to know about the topology in a bit more detail than just knowing how many batteries are there

I suggest to still start with raising an error if thats the simplest solution. The error will force the user to investigate the setup but thats OK. This is anyhow not a use case I think we'll see so often. Hence I would go for the adaptable approach later in time. Just my two cents....

llucax commented 1 year ago

Yeah, sounds reasonable to me. :+1:

Marenz commented 1 year ago

we could make the raising error the default and having a c'tor switch or so that makes it adapt as necessary?

shsms commented 1 year ago

I think the suggestions from Tiyash and Luca, of providing additional methods to identify siblings is the safer approach. But this can be done later.

I think we should always raise, because otherwise, if people make two battery pools - one with battery one and another with battery two, they would both be the same pool, which is an ambiguity we should avoid. People will use the flag without thinking and will shoot themselves in the foot.

llucax commented 1 year ago

The raised error could have the information about the siblings, so the use could capture it and re-request the the battery pool either including all the siblings, or none of them. But we could have a specific function too.