Skip to content

Fix unstable connectable ordering #3786

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.

Sign up for GitHub

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

Open
quinarygio wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Fix unstable connectable ordering #3786

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
184f97b
Add connectable ordering unit test
flo-dup Nov 28, 2025
275a016
Add successful connectable ordering unit test
quinarygio Jan 21, 2026
db36e3c
Add identifiable sort index
quinarygio Feb 18, 2026
42cc225
Implement sorting on network write
quinarygio Feb 23, 2026
49ac905
Fix test reference files
quinarygio Feb 27, 2026
e18c7a6
Fix after review
quinarygio Feb 27, 2026
73702cd
Merge branch 'main' into 3674-unstable-connectable-ordering
quinarygio Feb 27, 2026
1ee0b69
Fix after merge
quinarygio Feb 27, 2026
ccf8384
Handle feedbacks
alicecaron Mar 3, 2026
c15bfe5
Add TCK test on Identifiable getSortedIndex method
alicecaron Mar 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions iidm/iidm-api/src/main/java/com/powsybl/iidm/network/Identifiable.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,13 @@ default Network getParentNetwork() {
*/
String getId();

/**
* Get the sort index of the object.
*/
long getSortIndex();
Copy link
Member

@olperr1 olperr1 Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A test should be added in the TCK to ensure that the custom IIDM implementations handle it correctly.

Copy link
Contributor

@alicecaron alicecaron Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done


void setSortIndex(long index);

/**
* Get the aliases of the object.
*/
Expand Down
11 changes: 11 additions & 0 deletions iidm/iidm-impl/src/main/java/com/powsybl/iidm/network/impl/AbstractIdentifiable.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ abstract class AbstractIdentifiable<I extends Identifiable<I>> extends AbstractE

private final Set<String> aliasesWithoutType = new HashSet<>();
private final Map<String, String> aliasesByType = new HashMap<>();
private long sortIndex;

AbstractIdentifiable(String id, String name) {
this.id = id;
Expand Down Expand Up @@ -219,6 +220,16 @@ public Set<String> getPropertyNames() {
return properties.getPropertyNames();
}

@Override
public long getSortIndex() {
return sortIndex;
}

@Override
public void setSortIndex(long sortIndex) {
this.sortIndex = sortIndex;
}

@Override
public String toString() {
return id;
Expand Down
2 changes: 2 additions & 0 deletions iidm/iidm-impl/src/main/java/com/powsybl/iidm/network/impl/NetworkIndex.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ class NetworkIndex {

private final Map<String, Identifiable<?>> objectsById = new HashMap<>();
private final Map<String, String> idByAlias = new HashMap<>();
private long sortIndex = 0;

private final Map<Class<? extends Identifiable>, Set<Identifiable<?>>> objectsByClass = new HashMap<>();

Expand All @@ -43,6 +44,7 @@ void checkAndAdd(Identifiable<?> obj) {
+ ") '" + obj.getId() + "' already exists");
}
objectsById.put(obj.getId(), obj);
obj.setSortIndex(sortIndex++);
obj.getAliases().forEach(alias -> addAlias(obj, alias));

Set<Identifiable<?>> all = objectsByClass.computeIfAbsent(obj.getClass(), k -> new LinkedHashSet<>());
Expand Down
12 changes: 12 additions & 0 deletions iidm/iidm-impl/src/test/java/com/powsybl/iidm/network/impl/VariantManagerImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ private static final class IdentifiableMock extends AbstractExtendable<Identifia

private int reducedCount = 0;

private long sortIndex;

private IdentifiableMock(String id) {
this.id = id;
}
Expand Down Expand Up @@ -141,6 +143,16 @@ public void allocateVariantArrayElement(int[] indexes, int sourceIndex) {
public IdentifiableType getType() {
return null;
}

@Override
public long getSortIndex() {
return sortIndex;
}

@Override
public void setSortIndex(long index) {
this.sortIndex = index;
}
}

private static final class LoadExtension extends AbstractExtension<Load> implements MultiVariantObject {
Expand Down
4 changes: 3 additions & 1 deletion iidm/iidm-serde/src/main/java/com/powsybl/iidm/serde/util/IidmSerDeUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,9 @@ public static <T extends Identifiable> Iterable<T> sorted(Iterable<T> identifiab
return exportOptions.isSorted() ? StreamSupport.stream(identifiables.spliterator(), false)
.sorted(Comparator.comparing(Identifiable::getId))
.collect(Collectors.toList())
: identifiables;
: StreamSupport.stream(identifiables.spliterator(), false)
.sorted(Comparator.comparing(Identifiable::getSortIndex))
.collect(Collectors.toList());
Copy link
Member

@olperr1 olperr1 Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only the comparator is different between the 2 alternatives. It would be cleaner to choose the comparator first and then to execute the processing with it.

Copy link
Contributor

@alicecaron alicecaron Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

}

/**
Expand Down
37 changes: 37 additions & 0 deletions iidm/iidm-serde/src/test/java/com/powsybl/iidm/serde/ConnectableOrderingXmlTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/**
* Copyright (c) 2025, RTE (http://www.rte-france.com)
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/.
* SPDX-License-Identifier: MPL-2.0
*/
package com.powsybl.iidm.serde;

import com.powsybl.iidm.network.Network;
import org.junit.jupiter.api.Test;

import java.io.IOException;

import static org.junit.jupiter.api.Assertions.assertEquals;

/**
* @author Florian Dupuy {@literal <florian.dupuy at rte-france.com>}
*/
class ConnectableOrderingXmlTest extends AbstractIidmSerDeTest {

@Test
void testAttributOrderingKO() throws IOException {
ExportOptions exportOptions = new ExportOptions();
roundTripTest(Network.read("/twtOrdering.xiidm", getNetworkAsStream("/twtOrdering.xiidm")),
(n, p) -> NetworkSerDe.write(n, exportOptions, p),
NetworkSerDe::validateAndRead,
"/twtOrdering.xiidm");
}

@Test
void testCopy() throws IOException {
Network network = Network.read("/twtOrdering.xiidm", getNetworkAsStream("/twtOrdering.xiidm"));
Network exportNetwork = NetworkSerDe.copy(network);
assertEquals(exportNetwork.getTwoWindingsTransformers().toString(), network.getTwoWindingsTransformers().toString());
}
}
22 changes: 22 additions & 0 deletions iidm/iidm-serde/src/test/resources/twtOrdering.xiidm
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<?xml version="1.0" encoding="UTF-8"?>
<iidm:network xmlns:iidm="http://www.powsybl.org/schema/iidm/1_15" id="sim1" caseDate="2013-01-15T18:45:00.000+01:00" forecastDistance="0" sourceFormat="test" minimumValidationLevel="STEADY_STATE_HYPOTHESIS">
<iidm:substation id="P1" country="FR" tso="RTE" geographicalTags="A">
<iidm:voltageLevel id="VLGEN" nominalV="24.0" topologyKind="BUS_BREAKER">
<iidm:busBreakerTopology>
<iidm:bus id="NGEN1"/>
<iidm:bus id="NGEN2"/>
</iidm:busBreakerTopology>
<iidm:generator id="GEN" energySource="OTHER" minP="-9999.99" maxP="9999.99" voltageRegulatorOn="true" targetP="607.0" targetV="24.5" targetQ="301.0" bus="NGEN1" connectableBus="NGEN1">
<iidm:minMaxReactiveLimits minQ="-9999.99" maxQ="9999.99"/>
</iidm:generator>
</iidm:voltageLevel>
<iidm:voltageLevel id="VLHV1" nominalV="380.0" topologyKind="BUS_BREAKER">
<iidm:busBreakerTopology>
<iidm:bus id="NHV1"/>
</iidm:busBreakerTopology>
</iidm:voltageLevel>
<iidm:twoWindingsTransformer id="NGEN_NHV1" r="0.2" x="11.1" g="0.0" b="0.0" ratedU1="24.0" ratedU2="400.0" bus1="NGEN1" connectableBus1="NGEN1" voltageLevelId1="VLGEN" bus2="NHV1" connectableBus2="NHV1" voltageLevelId2="VLHV1"/>
<iidm:twoWindingsTransformer id="NGEN_NHV2" r="0.2" x="11.1" g="0.0" b="0.0" ratedU1="24.0" ratedU2="400.0" bus1="NGEN2" connectableBus1="NGEN2" voltageLevelId1="VLGEN" bus2="NHV1" connectableBus2="NHV1" voltageLevelId2="VLHV1"/>
<iidm:twoWindingsTransformer id="twtTest" r="1.0" x="1.0" g="0.0" b="0.0" ratedU1="220.0" ratedU2="400.0" bus1="NGEN1" connectableBus1="NGEN1" voltageLevelId1="VLGEN" bus2="NHV1" connectableBus2="NHV1" voltageLevelId2="VLHV1"/>
</iidm:substation>
</iidm:network>
22 changes: 22 additions & 0 deletions iidm/iidm-serde/src/test/resources/twtOrderingOk.xiidm
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<?xml version="1.0" encoding="UTF-8"?>
<iidm:network xmlns:iidm="http://www.powsybl.org/schema/iidm/1_15" id="sim1" caseDate="2013-01-15T18:45:00.000+01:00" forecastDistance="0" sourceFormat="test" minimumValidationLevel="STEADY_STATE_HYPOTHESIS">
<iidm:substation id="P1" country="FR" tso="RTE" geographicalTags="A">
<iidm:voltageLevel id="VLGEN" nominalV="24.0" topologyKind="BUS_BREAKER">
<iidm:busBreakerTopology>
<iidm:bus id="NGEN1"/>
<iidm:bus id="NGEN2"/>
</iidm:busBreakerTopology>
<iidm:generator id="GEN" energySource="OTHER" minP="-9999.99" maxP="9999.99" voltageRegulatorOn="true" targetP="607.0" targetV="24.5" targetQ="301.0" bus="NGEN1" connectableBus="NGEN1">
<iidm:minMaxReactiveLimits minQ="-9999.99" maxQ="9999.99"/>
</iidm:generator>
</iidm:voltageLevel>
<iidm:voltageLevel id="VLHV1" nominalV="380.0" topologyKind="BUS_BREAKER">
<iidm:busBreakerTopology>
<iidm:bus id="NHV1"/>
</iidm:busBreakerTopology>
</iidm:voltageLevel>
<iidm:twoWindingsTransformer id="NGEN_NHV1" r="0.2" x="11.1" g="0.0" b="0.0" ratedU1="24.0" ratedU2="400.0" voltageLevelId1="VLGEN" bus1="NGEN1" connectableBus1="NGEN1" voltageLevelId2="VLHV1" bus2="NHV1" connectableBus2="NHV1"/>
<iidm:twoWindingsTransformer id="twtTest" r="1.0" x="1.0" g="0.0" b="0.0" ratedU1="220.0" ratedU2="400.0" voltageLevelId1="VLGEN" bus1="NGEN1" connectableBus1="NGEN1" voltageLevelId2="VLHV1" bus2="NHV1" connectableBus2="NHV1"/>
<iidm:twoWindingsTransformer id="NGEN_NHV2" r="0.2" x="11.1" g="0.0" b="0.0" ratedU1="24.0" ratedU2="400.0" voltageLevelId1="VLGEN" bus1="NGEN2" connectableBus1="NGEN2" voltageLevelId2="VLHV1" bus2="NHV1" connectableBus2="NHV1"/>
</iidm:substation>
</iidm:network>
Loading