Support disconnection of DC elements in the load flow - #1471
Conversation
Signed-off-by: b.perreyon <baptiste.perreyon@supergrid-institute.com>
Signed-off-by: b.perreyon <baptiste.perreyon@supergrid-institute.com>
Signed-off-by: b.perreyon <baptiste.perreyon@supergrid-institute.com>
Signed-off-by: b.perreyon <baptiste.perreyon@supergrid-institute.com>
Signed-off-by: b.perreyon <baptiste.perreyon@supergrid-institute.com>
Signed-off-by: b.perreyon <baptiste.perreyon@supergrid-institute.com>
SGI-Landry
left a comment
There was a problem hiding this comment.
Thanks for this activity @bperr !
There are a few minor comments here and there. I think however that we should discuss tests in AcDcLoadFlowWithDisconnectionTest.
| } | ||
|
|
||
| /** | ||
| * Verify the DC node voltage is coherent with the nominal voltage |
There was a problem hiding this comment.
| * Verify the DC node voltage is coherent with the nominal voltage | |
| * Verify the DC node voltage is consistent with the nominal voltage |
But is this really needed? I would expect that you should know what values to expect precisely from your test cases, rather than test that the value is finite and "small enough".
| import static org.junit.jupiter.api.Assertions.*; | ||
|
|
||
| /** | ||
| * @author Baptiste Perreyon {@literal <bapstiste.perreyon at supergrid-institute.com>} |
There was a problem hiding this comment.
| * @author Baptiste Perreyon {@literal <bapstiste.perreyon at supergrid-institute.com>} | |
| * @author Baptiste Perreyon {@literal <baptiste.perreyon at supergrid-institute.com>} |
| * @author Baptiste Perreyon {@literal <bapstiste.perreyon at supergrid-institute.com>} | ||
| */ | ||
| @ExtendWith(ServiceParameterResolver.class) | ||
| class AcDcLoadFlowWithDisconnectionTest { |
There was a problem hiding this comment.
This can be simplified and made more robust. Can we please write down what behaviors to expect in a if condition then expectation way, and create just these tests, over observable behaviors? This should rely on analytical cases when this is numerical, with comparisons to expected values.
| // lcc converter not implemented yet | ||
| } else { | ||
| // Remove previous load flow results (we don't want the user to believe this are results from the new load flow) | ||
| acDcConverter.unsetSolvedValues(); |
There was a problem hiding this comment.
Very good point. However, a converter whose DC terminals are both disconnected belongs to no DcBus so it is never added to the acDcConverterSet in the first place.
Same issue with DcLines disconnected at both ends.
There was a problem hiding this comment.
Done after the load flow in Networks.resetState as all other equipments
| * | ||
| * @author Baptiste Perreyon {@literal <baptiste.perreyon at supergrid-institute.com>} | ||
| */ | ||
| final class DcNetworkValidationHelpers { |
There was a problem hiding this comment.
Good call to factor out these static functions. I am wondering if it is possible to simplify this code a bit. Maybe extract the topology visitor, and replace the predicate on converters by a boolean stating whether the test is needed or not?
Signed-off-by: b.perreyon <baptiste.perreyon@supergrid-institute.com>
Signed-off-by: b.perreyon <baptiste.perreyon@supergrid-institute.com>
Signed-off-by: b.perreyon <baptiste.perreyon@supergrid-institute.com>
Signed-off-by: b.perreyon <baptiste.perreyon@supergrid-institute.com>
Signed-off-by: b.perreyon <baptiste.perreyon@supergrid-institute.com>
Signed-off-by: b.perreyon <baptiste.perreyon@supergrid-institute.com>
Signed-off-by: b.perreyon <baptiste.perreyon@supergrid-institute.com>
SGI-Landry
left a comment
There was a problem hiding this comment.
Thanks for largely simplifying tests! Can you please make sure that the behaviors you want to test are perfectly clear? I suggest writing them down in comments as if condition then expectation. Then you will be sure you are testing exactly what you want.
| double dcLineR = network.getDcLine("dl34").getR(); // 0.1 Ohm | ||
| double expectedDcCurrent = 125; // 50 MW/400kV = 125A | ||
| // Check DC current | ||
| assertEquals(-expectedDcCurrent, network.getVoltageSourceConverter("conv23").getDcTerminal1().getI(), 1e-2); |
There was a problem hiding this comment.
How is the tolerance chosen?
1e-2 absolute ~ 1e-5 relative. Not bad, but depending on convergence parameters we can probably do better.
What about
- checking the convergence parameters to decide on the relative tolerance.
- factor it in a named constant, and document the choice.
There was a problem hiding this comment.
1e^-2 is the default tolerance in OLF for Newton-Raphson.
I imported it in the test from NewtonRaphsonStoppingCriteria.java in which it is defined in p.u.
|
|
||
| @Test | ||
| void dcLineTotalDisconnection() { | ||
| /// Same case study as dcLinePartialDisconnection. However, both terminals of the DC line are disconnected. |
There was a problem hiding this comment.
There seems to be a copy-paste error here. Code is inconsistent with the comment.
May I suggest to do this test at the end of the previous function? We would have
- Results with both lines connected.
- Results with one line disconnected.
- Results with both lines disconnected.
There was a problem hiding this comment.
The difference between the two test is whether the DC line is disconnected on one side or on both side; not whether there is one or two lines disconnected.
However its true that running the load flow on the fully connected network in the two tests brings no added value. I'll merge the two tests
| } | ||
|
|
||
| @Test | ||
| void dcLineDisconnectionLeadsToZeroCurrentInConverter() { |
There was a problem hiding this comment.
I don't understand the difference with the previous test cases. Does it make sense to regroup them? I miss the general intent. What is your activity supposed to implement and what simple behaviors do you want to check?
There was a problem hiding this comment.
In the previous test case, the disconnection of the DC line did not prevent the current to pass in the DC network (because there was a second DC line).
Here the disconnection of the DC line isolates the positive layer and triggers the automatic update of a converter's control mode. Additionnaly there is no DC current in this converter so its AC consumption corresponds to its idle loss
There was a problem hiding this comment.
You are making a good point. It would be very helpful to put this in your comments. Does this mean that you are testing "isolate the pole of a VSC => trigger automatic update of control mode" and maybe "=> zero DC current"? Then is it possible to check it more directly?
There was a problem hiding this comment.
I reformulated the comment hoping it is clearer now, but this was already mentioned.
In the test, I already check that the DC current in the isolated converter is 0.
As for the automatic update of the control mode, the IIDM is left untouched, so we cannot check it directly. However, what is already tested is that the voltage difference between the two DC buses of the converter is exactly 400 kV (the nominal voltage, set as targetVdc when the control mode of the converter is updated)
| @Test | ||
| void testAcDcConverterDisconnectionLeadsToZeroCurrentInNearbyIsolatedDcLines() { | ||
| /// When disconnecting the converter, the current in the DC line at the disconnected pole (positive) should be | ||
| /// null, but the current should still pass through the negative and neutral pole, due to the second converter. |
There was a problem hiding this comment.
I would state the expected behavior as something like
Disconnecting a terminal of a VSC removes the equations from its branch but not of the others.
Is this what you want to test?
There was a problem hiding this comment.
The network is a bipolar connection with DMR. When removing a converter with positive and neutral layer, the positive layer is isolated and no current can pass through it. However, the neutral layer is still connected to the negative layer, so the current can pass here. This is what this test verifies.
I don't understand what you mean by 'branch' here so I am not confortable with your proposal.
Signed-off-by: b.perreyon <baptiste.perreyon@supergrid-institute.com>
Signed-off-by: b.perreyon <baptiste.perreyon@supergrid-institute.com>
| } | ||
|
|
||
| @Test | ||
| void dcLineDisconnectionLeadsToZeroCurrentInConverter() { |
There was a problem hiding this comment.
You are making a good point. It would be very helpful to put this in your comments. Does this mean that you are testing "isolate the pole of a VSC => trigger automatic update of control mode" and maybe "=> zero DC current"? Then is it possible to check it more directly?
Signed-off-by: b.perreyon <baptiste.perreyon@supergrid-institute.com>
Signed-off-by: b.perreyon <baptiste.perreyon@supergrid-institute.com>
There was a problem hiding this comment.
Thank you
This is a quite large PR with more features that the PR title and description suggests, please break it down because it makes reviewing difficult. for example:
- import also DC components in LfNetwork (my other comment, is there any reason to not just use the islands computed by IIDM?)
- the "Sanity checks : detecting invalid DC configuration and automatically resolving reference-less islands"
- a fix to set unsolved values (Networks.java)
- support of disconnected elements
also please check for sonar (coverage and issues).
| /** | ||
| * The set of DC buses reachable from a DC bus by following connected DC lines, together with | ||
| * the connected DC grounds and the AC-DC converters attached to them. Built by breadth-first search. | ||
| */ | ||
| private static final class DcIsland implements DcTopologyVisitor { |
There was a problem hiding this comment.
I am wondering why this is needed, all topology processing: AC islands, DC islands, Connected AC/DC islands are already computed within IIDM.
LfNetwork-s are built from IIDM connected component-s
LfSynchronousNetwork are built from IIDM synchronous component-s
Instead of recomputing islands because of information lost at LfNetwork loading time, I think you should create for example LfDcNetwork from IIDM DC component-s at LfNetwork loading time.
There was a problem hiding this comment.
I realise the name of this class conflicts with the DC Island concept (I only had DC component wording in mind, sorry), and the docstring may be a bit unclear.
Here the only elements used to navigate between DC buses are DC lines. Therefore, one can only reach DC buses of the same layer (positive/neutral/negative) which is different from the DC island computed in IIDM. I'll refactor this to make it clear.
Side note: This process is done using the original Network object not the LfNetwork, so no information has been lost at this stage.
There was a problem hiding this comment.
I would add that when a DC line is disconnected, a "layer" may be split in two : the DC buses are not connected via the DC line anymore so their voltage are independent (so we need an element imposing it for each of them). However they are still part of the same DC component (because they are indireclty connected by converters and lines of another layer)
|
Hi @jeandemanged , thanks for your review ! |
Signed-off-by: b.perreyon <baptiste.perreyon@supergrid-institute.com>
Signed-off-by: b.perreyon <baptiste.perreyon@supergrid-institute.com>
|
As suggested, I have split this PR:
|
|



Please check if the PR fulfills these requirements
Does this PR already have an issue describing the problem?
No
What kind of change does this PR introduce?
Feature
What is the current behavior?
The AC-DC load flow did not consider wheher DC terminals of elements were connected or disconnected.
What is the new behavior (if this is a feature change)?
Disconnnected elements are properly taken into account during load flow
Does this PR introduce a breaking change or deprecate an API?
If yes, please check if the following requirements are fulfilled
What changes might users need to make in their application due to this PR? (migration steps)
Other information: