Skip to content

Fix out of substation voltage level should not hinder geo search #155

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
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Fix out of substation voltage level should not hinder geo search #155

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
854bd61
add a test with a vl without substation
laurgarn May 28, 2025
a0096b7
fix simple out of substation voltage level should not hinder geo data…
laurgarn May 28, 2025
b566a08
remove new unread variables
laurgarn May 28, 2025
8b63eb6
correct typo in comment
laurgarn Jun 2, 2025
8af410e
correct load strategy consistency
laurgarn Jun 3, 2025
90575b1
sonar happier
laurgarn Jun 4, 2025
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
10 changes: 8 additions & 2 deletions src/main/java/org/gridsuite/geodata/server/GeoDataController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,10 @@ public ResponseEntity<List<SubstationGeoData>> getSubstations(@Parameter(descrip
@Parameter(description = "Countries") @RequestParam(name = "country", required = false) List<String> countries,
@RequestBody(required = false) List<String> substationIds) {
Set<Country> countrySet = toCountrySet(countries);
Network network = networkStoreService.getNetwork(networkUuid, substationIds != null ? PreloadingStrategy.NONE : PreloadingStrategy.COLLECTION);
PreloadingStrategy preloadingStrategy = geoDataService.preferPreload(substationIds) ?
PreloadingStrategy.COLLECTION :
PreloadingStrategy.NONE;
Network network = networkStoreService.getNetwork(networkUuid, preloadingStrategy);
if (variantId != null) {
network.getVariantManager().setWorkingVariant(variantId);
}
Expand All @@ -74,7 +77,10 @@ public ResponseEntity<List<LineGeoData>> getLines(@Parameter(description = "Netw
@Parameter(description = "Countries") @RequestParam(name = "country", required = false) List<String> countries,
@RequestBody(required = false) List<String> lineIds) {
Set<Country> countrySet = toCountrySet(countries);
Network network = networkStoreService.getNetwork(networkUuid, lineIds != null ? PreloadingStrategy.NONE : PreloadingStrategy.COLLECTION);
PreloadingStrategy preloadingStrategy = geoDataService.preferPreload(lineIds) ?
PreloadingStrategy.COLLECTION :
PreloadingStrategy.NONE;
Network network = networkStoreService.getNetwork(networkUuid, preloadingStrategy);
if (variantId != null) {
network.getVariantManager().setWorkingVariant(variantId);
}
Expand Down
60 changes: 40 additions & 20 deletions src/main/java/org/gridsuite/geodata/server/GeoDataService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -385,12 +385,11 @@ private static Map<String, Set<String>> getNeighbours(List<Substation> substatio
for (Substation s : substations) {
for (VoltageLevel vl : s.getVoltageLevels()) {
for (Line line : vl.getConnectables(Line.class)) {
Substation s1 = line.getTerminal1().getVoltageLevel().getSubstation().orElseThrow(); // TODO
Substation s2 = line.getTerminal2().getVoltageLevel().getSubstation().orElseThrow(); // TODO
if (s1 != s) {
neighbours.get(s.getId()).add(s1.getId());
} else if (s2 != s) {
neighbours.get(s.getId()).add(s2.getId());
Substation s1 = line.getTerminal1().getVoltageLevel().getSubstation().orElse(null);
Substation s2 = line.getTerminal2().getVoltageLevel().getSubstation().orElse(null);
Substation otherSide = s1 == s ? s2 : s1;
if (otherSide != null) {
neighbours.get(s.getId()).add(otherSide.getId());
}
}
}
Expand Down Expand Up @@ -445,7 +444,8 @@ boolean emptyOrEquals(String emptyable, String s) {
* <p>
* returns null when the substations at the end of the line are missing.
*/
private LineGeoData getLineGeoDataWithEndSubstations(Map<String, LineGeoData> linesGeoDataDb, Map<String, SubstationGeoData> substationGeoDataDb, String lineId, Substation substation1, Substation substation2) {
private LineGeoData getLineGeoDataWithEndSubstations(Map<String, LineGeoData> linesGeoDataDb,
Map<String, SubstationGeoData> substationGeoDataDb, String lineId, Substation substation1, Substation substation2) {
LineGeoData geoData = linesGeoDataDb.get(lineId);
SubstationGeoData substation1GeoData = substationGeoDataDb.get(substation1.getId());
SubstationGeoData substation2GeoData = substationGeoDataDb.get(substation2.getId());
Expand Down Expand Up @@ -515,17 +515,25 @@ List<LineGeoData> getLinesByCountries(Network network, Set<Country> countries) {

// we also want the destination substation (so we add the neighbouring country)
Set<Country> countryAndNextTo = mapSubstationsByLine.entrySet().stream().flatMap(entry ->
Stream.of(entry.getValue().getLeft(), entry.getValue().getRight()).map(Substation::getNullableCountry).filter(Objects::nonNull)).collect(Collectors.toSet());
Stream.of(entry.getValue().getLeft(), entry.getValue().getRight())
.filter(Objects::nonNull)
.map(Substation::getNullableCountry)
.filter(Objects::nonNull)).collect(Collectors.toSet());

Map<String, SubstationGeoData> substationGeoDataDb = getSubstationMapByCountries(network, countryAndNextTo);
List<LineGeoData> geoData = new ArrayList<>();

mapSubstationsByLine.forEach((key, value) -> {
LineGeoData geo = getLineGeoDataWithEndSubstations(linesGeoDataDb, substationGeoDataDb, key, value.getLeft(), value.getRight());
if (geo != null) {
geoData.add(geo);
}
});
mapSubstationsByLine.entrySet().stream()
.filter(entry -> entry.getValue() != null)
.filter(entry -> entry.getValue().getLeft() != null)
.filter(entry -> entry.getValue().getRight() != null)
Copy link
Contributor

Choose a reason for hiding this comment

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

Only one filter call instead of 3 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see it clearer that way. Do you think it could have a noticeable influence on performance ?

.forEach(entry -> {
LineGeoData geo = getLineGeoDataWithEndSubstations(linesGeoDataDb, substationGeoDataDb, entry.getKey(),
entry.getValue().getLeft(), entry.getValue().getRight());
if (geo != null) {
geoData.add(geo);
}
});

LOGGER.info("{} lines read from DB in {} ms", linesGeoDataDb.size(), stopWatch.getTime(TimeUnit.MILLISECONDS));

Expand All @@ -534,8 +542,8 @@ List<LineGeoData> getLinesByCountries(Network network, Set<Country> countries) {

private Pair<Substation, Substation> getSubstations(Identifiable<?> identifiable) {
return switch (identifiable.getType()) {
case LINE -> Pair.of(((Line) identifiable).getTerminal1().getVoltageLevel().getSubstation().orElseThrow(),
((Line) identifiable).getTerminal2().getVoltageLevel().getSubstation().orElseThrow());
case LINE -> Pair.of(((Line) identifiable).getTerminal1().getVoltageLevel().getSubstation().orElse(null),
((Line) identifiable).getTerminal2().getVoltageLevel().getSubstation().orElse(null));
case TIE_LINE ->
Pair.of(((TieLine) identifiable).getDanglingLine1().getTerminal().getVoltageLevel().getSubstation().orElseThrow(),
((TieLine) identifiable).getDanglingLine2().getTerminal().getVoltageLevel().getSubstation().orElseThrow());
Expand All @@ -546,16 +554,24 @@ private Pair<Substation, Substation> getSubstations(Identifiable<?> identifiable
};
}

public boolean preferPreload(List<String> ids) {
return ids == null || ids.size() > 4;
}

@Transactional(readOnly = true)
public List<SubstationGeoData> getSubstationsData(Network network, Set<Country> countrySet, List<String> substationIds) {
CompletableFuture<List<SubstationGeoData>> substationGeoDataFuture = geoDataExecutionService.supplyAsync(() -> {
if (substationIds != null) {
if (substationIds != null && substationIds.size() < 4) {
if (!countrySet.isEmpty()) {
LOGGER.warn("Countries will not be taken into account to filter substation position.");
}
return getSubstationsByIds(network, new HashSet<>(substationIds));
} else {
return getSubstationsByCountries(network, countrySet);
List<SubstationGeoData> substationsByCountries = getSubstationsByCountries(network, countrySet);
if (substationIds != null && !substationIds.isEmpty()) {
return substationsByCountries.stream().filter(s -> substationIds.contains(s.getId())).toList();
}
return substationsByCountries;
}
});
try {
Expand All @@ -571,13 +587,17 @@ public List<SubstationGeoData> getSubstationsData(Network network, Set<Country>
@Transactional(readOnly = true)
public List<LineGeoData> getLinesData(Network network, Set<Country> countrySet, List<String> lineIds) {
CompletableFuture<List<LineGeoData>> lineGeoDataFuture = geoDataExecutionService.supplyAsync(() -> {
if (lineIds != null) {
if (!preferPreload(lineIds)) {
if (!countrySet.isEmpty()) {
LOGGER.warn("Countries will not be taken into account to filter line position.");
}
return getLinesByIds(network, new HashSet<>(lineIds));
} else {
return getLinesByCountries(network, countrySet);
List<LineGeoData> linesByCountries = getLinesByCountries(network, countrySet);
if (lineIds != null && !lineIds.isEmpty()) {
return linesByCountries.stream().filter(ln -> lineIds.contains(ln.getId())).toList();
}
return linesByCountries;
}
});
try {
Expand Down
29 changes: 29 additions & 0 deletions src/test/java/org/gridsuite/geodata/server/GeoDataServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,7 @@ void testNonExisting() {
.setG2(0.0)
.setB2(386E-6 / 2)
.add();
// if not geo-data is associated to a substation, this substation should not be mentionned in the returned value
List<SubstationGeoData> substationsGeoData = geoDataService.getSubstationsByCountries(network, new HashSet<>(Collections.singletonList(Country.FR)));
assertFalse(substationsGeoData.stream().anyMatch(Objects::isNull), "Must not contain nulls");
assertFalse(substationsGeoData.stream().anyMatch(s -> notexistsub1.getId().equals(s.getId())),
Expand All @@ -258,6 +259,34 @@ void testNonExisting() {
"Must not contain unknown lines " + notexistline.getId());
}

@Test
void outOfSubstationVoltageLevelShouldNotHinderLineDataGathering() {
Network network = EurostagTutorialExample1Factory.create();
List<VoltageLevel> existingVls = network.getVoltageLevelStream().toList();
VoltageLevel p1 = network.newVoltageLevel()
.setId("PIQUAGE1").setNominalV(380).setTopologyKind(TopologyKind.BUS_BREAKER)
.add();
Bus bus1 = p1.getBusBreakerView().newBus()
.setId("NPIQ1")
.add();
network.newLine()
.setId("LINE_PIQ_AT_A_SIDE")
.setVoltageLevel1(existingVls.get(0).getId())
.setBus1(existingVls.get(0).getBusBreakerView().getBuses().iterator().next().getId())
.setConnectableBus1(existingVls.get(0).getBusBreakerView().getBuses().iterator().next().getId())
.setVoltageLevel2(p1.getId()).setBus2(bus1.getId()).setConnectableBus2(bus1.getId())
.setR(3.0)
.setX(33.0)
.setG1(0.0)
.setB1(386E-6 / 2)
.setG2(0.0)
.setB2(386E-6 / 2)
.add();

List<LineGeoData> linesGeoData = geoDataService.getLinesByCountries(network, new HashSet<>(Collections.singletonList(Country.FR)));
assertEquals(2, linesGeoData.size());
}

@Test
void testSimilarNeighborhoodOffset() {
Network network = EurostagTutorialExample1Factory.create();
Expand Down