Mapping branches in new UCT by Sebasss-h · Pull Request #6 · rte-france/Transmission-Reliability-Margin-Algorithm

Sebasss-h · 2025-04-22T08:57:34Z

No description provided.

Simplified the logic for multi-line mappings by restructuring the test case to use streamlined mapping methods and improved assertions. Added clear `Given-When-Then` test structure, enhancing code readability and maintainability.

Replaced direct access to elementName with Optional to prevent potential null pointer exceptions. This ensures safer handling when retrieving and processing the property from networkMarketBased. Default values are used if elementName is absent.

# Conflicts: # src/test/java/com/rte_france/trm_algorithm/UcteMappingTest.java

terriervik · 2025-05-06T09:06:02Z

@murgeyseb FYI

terriervik

Thanks for the PR, please see my comments

terriervik · 2025-06-04T14:52:09Z

@@ -0,0 +1,87 @@
+/*
+ * Copyright (c) 2024, RTE (http://www.rte-france.com)


Should this copyright be 2025 (same for all new classes in this PR)?

(@murgeyseb I don't know the correct policy)

terriervik · 2025-06-04T15:15:40Z

+    }
+
+    @Test
+    void testRealNetwork() {


This method should be deleted.

terriervik · 2025-06-04T15:16:02Z

+    }
+
+    @Test
+    void testNewMappingMap() {


Add a test to this method (assertSomething), or delete it.

terriervik · 2025-06-04T15:22:22Z

+                new MappingResults("FFNHV311 FFNHV211 1", "FFNHV211 FFNHV311 1", true),
+                new MappingResults("FFNGEN71 FFNHV111 1", "FFNGEN71 FFNHV111 1", true),
+                new MappingResults("FFNHV211 FFNLOA31 L", "FFNHV211 FFNLOA31 L", true));
+        //assertEquals(expectedMappingResults, mappingResults.mappingResults());


You should add an assertSomething at the end of this method, otherwise it is not a test.

terriervik · 2025-06-04T15:38:36Z

+ * @author Sebastian Huaraca {@literal <sebastian.huaracalapa at rte-france.com>}
+ */
+
+public class UcteMappingTest {


A general comment on this test class: could you please describe (in comments) what you precisely test in each method?
In particular, I am curious to see written which test method checks the following functionalities:
A/ same substation code (first 7 characters) for side 1 and for side 2 + same order code
B/ same substation code (first 7 characters) for side 1 and for side 2, both sides are inverted between the files + same order code
C/ A and B, but with different order code and same element name
D/ Line from network 1 missing in network 2
E/ Line from network 1 pointing at several elements from network 2
F/ others, if I forgot something :)

terriervik · 2025-06-05T15:16:02Z

-            .filter(branchId -> Objects.isNull(marketBasedNetwork.getBranch(branchId)))
-            .sorted()
-            .toList();
+                .filter(branchId -> Objects.isNull(marketBasedNetwork.getBranch(branchId)))


Delete this whitespaces added on lines 51 to 53 and 72 to 79

The Java indentation should be tab size 4, indent 4 and continuation indent 4

murgeyseb · 2025-06-06T06:45:59Z

+        }
+    }
+
+    private static String getOrderCode(String id) {


What happen if the id does not respect UCTE formet (e.g. modified network using script, or CGMES based network). Fail ?

OpenSuze

Could you add some documentation in doc/algorithm.md ?

OpenSuze · 2025-06-06T06:43:07Z

            <groupId>com.powsybl</groupId>
            <artifactId>open-rao-crac-io-commons</artifactId>
            <version>${powsybl.rao.version}</version>
-            <scope>test</scope>


Could you add the default scope and move this dependency into its section (in alphabetical order: line 83 I think) ?

OpenSuze · 2025-06-06T06:44:45Z

-            .filter(branchId -> Objects.isNull(marketBasedNetwork.getBranch(branchId)))
-            .sorted()
-            .toList();
+                .filter(branchId -> Objects.isNull(marketBasedNetwork.getBranch(branchId)))


The Java indentation should be tab size 4, indent 4 and continuation indent 4

OpenSuze · 2025-06-06T07:02:08Z

File not used in tests ? Add a test or remove this file

OpenSuze · 2025-06-06T07:03:13Z

Why is there a test file in the ressources ?

OpenSuze · 2025-06-06T07:05:01Z

Add this image in the test documentation

Removed unnecessary blank line for cleaner code. Adjusted indentation in lambda function for better readability and consistency.

Sebasss-h added 6 commits April 16, 2025 17:26

UCTE mapping for one line

333c39c

Create MappingResults.java

0a77289

Check style

1acfea4

Fix

c709850

Test Multi Lines ?

b5fad73

Test Multi Lines

4089718

OpenSuze reviewed Apr 22, 2025

View reviewed changes

Sebasss-h and others added 3 commits April 24, 2025 11:39

Test Multi Lines elementName

a7b822a

OpenSuze reviewed Apr 24, 2025

View reviewed changes

Comment thread src/test/java/com/rte_france/trm_algorithm/UcteMappingTest.java Outdated

Sebasss-h added 4 commits April 24, 2025 17:00

Test Line Switch Position

8ffa76c

Merge remote-tracking branch 'origin/ucteMapping' into ucteMapping

4b74b1c

# Conflicts: # src/test/java/com/rte_france/trm_algorithm/UcteMappingTest.java

Test Weird Cases (missing line)

e398128

TieLines and Duplicate Values

9bf4a5e

terriervik requested review from murgeyseb and removed request for murgeyseb May 6, 2025 09:05

Sebasss-h added 12 commits May 6, 2025 17:00

Correction UCTEMapping

399bba1

Refactoring 1 UcteMapping

b249e63

Add importNetworkInPc and searching bugs

e1f6b01

Solving and detecting problems UcteMappingTest

836f2e5

Solving and detecting problems UcteMappingTest

fb4c94d

Solving and detecting problems testRealNetwork

6e2496b

Adding isFictitious filter

2c302bc

UcteMapping mapNetworks2

2138b81

UcteMapping mapNetworks2

cd70139

Merge branch 'main' into ucteMapping

1759760

UcteMapping line "clean"

5667b91

Removing impact Trm algorithm

81d3026

Sebasss-h added 7 commits May 23, 2025 15:05

Put private package visibility ucte mapping

1dead70

Fix country filtering incorrect implementation

6dc9252

Improve readability of the code and overall performance

c2c40f4

Improve return statements

845b1ff

DuplicateCheck method

49b1791

DuplicateCheck method in mapNetworks

1255c30

Refactor Uctemapping function

44fdc6b

terriervik requested changes Jun 4, 2025

View reviewed changes

UcteMapper function and fix-tests

5a1848e

terriervik reviewed Jun 5, 2025

View reviewed changes

Sebasss-h changed the title ~~[WIP] Mapping branches in new UCT~~ Mapping branches in new UCT Jun 5, 2025

murgeyseb requested changes Jun 6, 2025

View reviewed changes

OpenSuze requested changes Jun 6, 2025

View reviewed changes

Sebasss-h added 2 commits June 6, 2025 16:46

Fix tests and update code comments

a8caada

Implement UCteMapper in TrmAlgorithm and identify bug in test

5fe88bf

murgeyseb requested changes Jun 13, 2025

View reviewed changes

Comment thread src/main/java/com/rte_france/trm_algorithm/TrmAlgorithm.java

Sebasss-h and others added 3 commits June 13, 2025 16:19

Fix TrmAlgorithm

40137a1

rollback file modifications

739e123

Removed unnecessary blank line for cleaner code. Adjusted indentation in lambda function for better readability and consistency.

rollback file modifications

c422bf8

Removed unnecessary blank line for cleaner code. Adjusted indentation in lambda function for better readability and consistency.

murgeyseb merged commit a0c8002 into main Jun 16, 2025
1 check passed

murgeyseb deleted the ucteMapping branch June 16, 2025 07:39

		@@ -0,0 +1,87 @@
		/*
		* Copyright (c) 2024, RTE (http://www.rte-france.com)

Conversation

Sebasss-h commented Apr 22, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

terriervik commented May 6, 2025

Uh oh!

terriervik left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

OpenSuze left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants