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.
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
Create network elements #259
Conversation
7b71862
to
078087f
Compare
SonarCloud Quality Gate failed. |
@EtienneLt it seems that your PR is needed for long-term studies. Do you think that you can rework on that in the next following months? It would be great! |
Signed-off-by: Etienne LESOT <etienne.lesot@rte-france.com>
Signed-off-by: Etienne LESOT <etienne.lesot@rte-france.com>
Signed-off-by: Etienne LESOT <etienne.lesot@rte-france.com>
Signed-off-by: Etienne LESOT <etienne.lesot@rte-france.com>
078087f
to
0e831fb
Compare
yes no problem |
- expose list of series metadata for element creation dataframes - isolate elements creation tests - add some missing creation (HVDC lines, substations) Signed-off-by: Sylvain Leclerc <sylvain.leclerc@rte-france.com>
Signed-off-by: Sylvain Leclerc <sylvain.leclerc@rte-france.com>
Signed-off-by: Sylvain Leclerc <sylvain.leclerc@rte-france.com>
Signed-off-by: Sylvain Leclerc <sylvain.leclerc@rte-france.com>
Signed-off-by: Sylvain Leclerc <sylvain.leclerc@rte-france.com>
Signed-off-by: Sylvain Leclerc <sylvain.leclerc@rte-france.com>
Signed-off-by: Sylvain Leclerc <sylvain.leclerc@rte-france.com>
Signed-off-by: Sylvain Leclerc <sylvain.leclerc@rte-france.com>
Signed-off-by: Sylvain Leclerc <sylvain.leclerc@rte-france.com>
Signed-off-by: Sylvain Leclerc <sylvain.leclerc@rte-france.com>
Signed-off-by: Sylvain Leclerc <sylvain.leclerc@rte-france.com>
Signed-off-by: Sylvain Leclerc <sylvain.leclerc@rte-france.com>
Signed-off-by: Sylvain Leclerc <sylvain.leclerc@rte-france.com>
Signed-off-by: Sylvain Leclerc <sylvain.leclerc@rte-france.com>
Signed-off-by: Sylvain Leclerc <sylvain.leclerc@rte-france.com>
Actually something is missing for node breaker topologies: |
Signed-off-by: Sylvain Leclerc <sylvain.leclerc@rte-france.com>
Signed-off-by: Sylvain Leclerc <sylvain.leclerc@rte-france.com>
Signed-off-by: Sylvain Leclerc <sylvain.leclerc@rte-france.com>
Signed-off-by: Sylvain Leclerc <sylvain.leclerc@rte-france.com>
Signed-off-by: Sylvain Leclerc <sylvain.leclerc@rte-france.com>
BatteryAdder batteryAdder = network.getVoltageLevel(dataframe.getStringValue("voltage_level_id", indexElement) | ||
.orElseThrow(() -> new PowsyblException("voltage_level_id is missing"))) | ||
.newBattery(); | ||
NetworkElementCreationUtils.createInjection(batteryAdder, dataframe, indexElement); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Be careful here: if not present, I think that you will have NaN values.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See discussion synthesis about default values in comment below.
|
||
private static final List<SeriesMetadata> METADATA = List.of( | ||
SeriesMetadata.stringIndex("id"), | ||
SeriesMetadata.strings("voltage_level_id"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a general question: when you only fill the bus id, and not the connectable bus id, what is happening ? And if you want a disconnected battery, you will have to fill the connectable bus only ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See discussion synthesis about default values in comment below.
public void addElement(Network network, UpdatingDataframe dataframe, int indexElement) { | ||
DanglingLineAdder adder = network.getVoltageLevel(dataframe.getStringValue("voltage_level_id", indexElement) | ||
.orElseThrow(() -> new PowsyblException("voltage_level_id is missing"))).newDanglingLine(); | ||
NetworkElementCreationUtils.createInjection(adder, dataframe, indexElement); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that we have to think about g
and a b
default values equals to zero.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See discussion synthesis about default values in comment below.
GeneratorAdder generatorAdder = network.getVoltageLevel(dataframe.getStringValue("voltage_level_id", indexElement) | ||
.orElseThrow(() -> new PowsyblException("voltage_level_id is missing"))) | ||
.newGenerator(); | ||
NetworkElementCreationUtils.createInjection(generatorAdder, dataframe, indexElement); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that we have to think about default values for minP and maxP (it seems to be NaN in powsybl-core), right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See discussion synthesis about default values in comment below.
NetworkElementCreationUtils.createIdentifiable(adder, dataframe, indexElement); | ||
dataframe.getStringValue("converter_station1_id", indexElement).ifPresent(adder::setConverterStationId1); | ||
dataframe.getStringValue("converter_station2_id", indexElement).ifPresent(adder::setConverterStationId2); | ||
dataframe.getDoubleValue("max_p", indexElement).ifPresent(adder::setMaxP); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Default value for maxP ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See discussion synthesis about default values in comment below.
public void addElement(Network network, UpdatingDataframe dataframe, int indexElement) { | ||
LineAdder lineAdder = network.newLine(); | ||
NetworkElementCreationUtils.createBranch(lineAdder, dataframe, indexElement); | ||
dataframe.getDoubleValue("b1", indexElement).ifPresent(lineAdder::setB1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that for line, we must think about by default having b1, b2, g1 and g2 equal to zero.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See discussion synthesis about default values in comment below.
private static final List<SeriesMetadata> METADATA = List.of( | ||
SeriesMetadata.stringIndex("id"), | ||
SeriesMetadata.strings("regulation_mode"), | ||
SeriesMetadata.doubles("target_deadband"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The target deadband could be at zero by default to ease understanding.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See discussion synthesis about default values in comment below.
SeriesMetadata.ints("tap") | ||
); | ||
|
||
private static final List<SeriesMetadata> STEPS_METADATA = List.of( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For phase tap changers, I think that we can provide a lot of default values and maybe allow only angles to be set.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See discussion synthesis about default values in comment below.
*/ | ||
public class RatioTapChangerDataframeAdder implements NetworkElementAdder { | ||
|
||
private static final List<SeriesMetadata> METADATA = List.of( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On load can be to true by default, and the target deadband could be to zero. For the ratio tap changer, I think that it could be great to have the possibility to set only ratios.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See discussion synthesis about default values in comment below.
* @author Sylvain Leclerc <sylvain.leclerc@rte-france.com> | ||
*/ | ||
public class ShuntDataframeAdder implements NetworkElementAdder { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general, for shunt, it could be great to have a way to only set a B from a dataframe, that will create a shunt understandable by Powsybl.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See discussion synthesis about default values in comment below.
*/ | ||
public class SwitchDataframeAdder extends AbstractSimpleAdder { | ||
|
||
private static final List<SeriesMetadata> METADATA = List.of( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general: it always has been strange to be to give both voltage level id and bus id. It will be very great to have the possibility to give only busId to create equipments in the network, and to have a way to retrieve the voltage level. It is a bit confusing to expose the complexity of IIDM in python.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See discussion synthesis about default values in comment below.
SeriesMetadata.doubles("rated_u1"), | ||
SeriesMetadata.doubles("rated_u2"), | ||
SeriesMetadata.doubles("rated_s"), | ||
SeriesMetadata.doubles("b"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same remark as lines. And maybe rated U1 and U2 could also have default values equal to nominal voltage at ends. What do you think ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See discussion synthesis about default values in comment below.
Just writing down the outcome of the discussions about default values: currently, we inherit the behaviour of powsybl-core adders. Therefore, unless there is something specific that the python community asks for, and that we really don't want to implement in powsybl-core, we'll keep it this way. |
Signed-off-by: Sylvain Leclerc <sylvain.leclerc@rte-france.com>
Signed-off-by: Sylvain Leclerc <sylvain.leclerc@rte-france.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am going to try to put some default values in the adders of powsybl-core, and we will see what remains for pypowsybl !
Kudos, SonarCloud Quality Gate passed! |
Please check if the PR fulfills these requirements (please use
'[x]'
to check the checkboxes, or submit the PR and then click the checkboxes)What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Feature.
What is the current behavior? (You can also link to an open issue here)
Network elements cannot be created through python methods.
What is the new behavior (if this is a feature change)?
We can now create network elements, for example: