powsybl-core icon indicating copy to clipboard operation
powsybl-core copied to clipboard

calculatedbus is not a real identifiable

Open jonenst opened this issue 2 years ago • 7 comments

  • Do you want to request a feature or report a bug? bug

  • What is the current behavior? calculatedbus inherits from identifiable

  • What is the expected behavior? calculatedbus is not an identifiable

  • What is the motivation / use case for changing the behavior? There are multiple things that make calculatedbus weird as identifiables:

  • can't get them with network.getIdentifiable(string id)
  • the ids are generated, so implementing the uniqueness constraint is hard when you want nice ids (especially when the network is only partially loaded, e.g. network-store)
  • .. other ?

We could split the Bus interface from identifiable so that ConfiguredBus has both identifiable and bus, while CalculatedBus only has bus.

When using the busbreakerview, Bus getBus() can return a configuredbus (busbreaker topology) or a calculatedbus (nodebreaker topology). Today, even though you can technically call the identifiable methods on both objects, it won't work, for example if you do network.getIdentifiable(bus.getId()) it will return the configured buses but null for the calculatedbus, so you will get NPEs down the line if you use this without care.

For the busview, it's always calculatedbus so we we could change the return type to calculated bus

jonenst avatar Jun 13 '22 15:06 jonenst

Agreed, actually I think that we should not expose only "views" in the API, but also the underlying structure, to make expectations on each topology kind more clear.

It means, like in iidm-impl, having two sub-interfaces of VoltageLevel (bus breaker and node breaker). Each one would expose only the corresponding methods (getNode, getBus ...). getBus would return explicitly a ConfiguredBus, but would keep both "views". At that point, it would make sense to remove modification methods from the views (newBus etc.), since they are actually supported by only one of the 2 views.

So we would have something like:

interface VoltageLevel {
    // Still same views, but trimmed of the modification methods
    BusView getBusView();
    BusBreakerView getBusBreakerView();
    // Not sure for node breaker view which makes sense only for one of the sub-interface
}

interface BusBreakerVoltageLevel extends VoltageLevel {
    // A real identifiable here
    ConfiguredBus getBus(String id);
    ...
    // Modification methods
    BusAdder newBus();
}


interface NodeBreakerVoltageLevel extends VoltageLevel {
    int[] getNodes();
    ...

}

sylvlecl avatar Jun 13 '22 15:06 sylvlecl

For information, we still need unicity IDs for two cases:

  • XIIDM export using a calculated view (for example, export of node-breaker in bus-breaker): in case of redundant IDs, the file generated by powsybl will throw an exception when re-imported... Please note that this is not an unusual case, it is the operational mode for several external loadflow. It can be bypassed by using a custom updater but it is not always ideal.
  • CGMES export where calculated bus IDs are exported and must be unique. This can also be bypassed by changing the naming strategy but it might be more costly (to be tested).

miovd avatar Jun 14 '22 08:06 miovd

  • the ids are generated, so implementing the uniqueness constraint is hard when you want nice ids (especially when the network is only partially loaded, e.g. network-store)

Actually is it that hard to ensure, since voltage level IDs are unique, and often used for the generation of bus IDs ?

sylvlecl avatar Jun 14 '22 08:06 sylvlecl

For the busview, it's always calculatedbus so we we could change the return type de calculated bus

Not sure it's a good idea: we could imagine later to have a third topology kind "bus branch", where the buses would be configured (like the "bus breaker" kind, but higher level without switches).

There have been requests in that direction, see https://github.com/powsybl/powsybl-core/issues/1773

sylvlecl avatar Jun 14 '22 08:06 sylvlecl

  • the ids are generated, so implementing the uniqueness constraint is hard when you want nice ids (especially when the network is only partially loaded, e.g. network-store)

Actually is it that hard to ensure, since voltage level IDs are unique, and often used for the generation of bus IDs ?

The naming strategy is too generic for now (voltageLevelId + _ + index) and sometimes it is how busbar sections are named (in some formats).

miovd avatar Jun 14 '22 11:06 miovd

Ah yes, I missed the risk of conflict with other types of network elements, thanks for the clarification

sylvlecl avatar Jun 14 '22 11:06 sylvlecl

That's what I meant with "nice" ids. It's easy to generate a uuid that will be unique. It's hard to generate something that a user will understand and not find weird and instantly recognize as an equivalent to configured bus ids (or busbar section ids). It's kind of a psychological issue too: if you have a network with 2 voltage levels: a nodebreaker vl VL1 and a busbarsection VL1_1 and a busbreaker voltage level VL2 with a configured bus VL2_1. Then you show the users a busview or a busbreaker view of the network, are they happy with "__VL1_1" and "VL2_1" ? "VL1_121ae1f7092fd31ab324bc" and "VL2_1" ? (I think the current code would generate "VL1_1#1" and "VL2_1")

jonenst avatar Jul 26 '22 15:07 jonenst