Skip to content
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

Discrete control optional #319

Merged
merged 11 commits into from
Jun 17, 2021
Merged

Discrete control optional #319

merged 11 commits into from
Jun 17, 2021

Conversation

flo-dup
Copy link
Contributor

@flo-dup flo-dup commented Jun 11, 2021

Please check if the PR fulfills these requirements (please use '[x]' to check the checkboxes, or submit the PR and then click the checkboxes)

  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

Does this PR already have an issue describing the problem ?
No

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Refactor

What is the current behavior? (You can also link to an open issue here)
LfBus::getDiscreteVoltageControl and LfBranch::getDiscreteVoltageControl return null if no control

What is the new behavior (if this is a feature change)?
the getDiscreteVoltageControl return an Optional<DiscreteVoltageControl>

Does this PR introduce a breaking change or deprecate an API?
No

Other information:
Done to have something similar to VoltageControl

@flo-dup flo-dup changed the base branch from master to issue-discrete-control June 11, 2021 07:31
@flo-dup flo-dup force-pushed the issue-discrete-control branch 2 times, most recently from 308216f to 92ff1f7 Compare June 11, 2021 13:13
@flo-dup flo-dup force-pushed the discrete-control-optional branch from 80f12a6 to e93032e Compare June 11, 2021 13:16
Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>
@flo-dup flo-dup force-pushed the discrete-control-optional branch from e93032e to 6e8e01a Compare June 15, 2021 13:28
@flo-dup flo-dup changed the base branch from issue-discrete-control to master June 15, 2021 14:05
annetill and others added 6 commits June 16, 2021 07:51
Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>
Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>
Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>
Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>
Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>
@flo-dup flo-dup requested a review from annetill June 16, 2021 08:28
@@ -35,11 +37,12 @@ public OuterLoopStatus check(OuterLoopContext context, Reporter reporter) {

if (context.getIteration() == 0) {
for (LfBus bus : context.getNetwork().getBuses()) {
if (bus.isDiscreteVoltageControlled()) {
Optional<DiscreteVoltageControl> dvc = bus.getDiscreteVoltageControl().filter(dvc0 -> bus.isDiscreteVoltageControlled());
if (dvc.isPresent()) {
Copy link
Member

@annetill annetill Jun 16, 2021

Choose a reason for hiding this comment

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

Can you hare change for a more explicit name such as voltageControl because you use it during several lines? And keep dvc for the filter?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

in fact I changed the code to have something similar to PhaseControlOuterLoop, which is clearer at the cost of creating a list of enabled voltage controls.

EquationTerm vTerm = EquationTerm.createVariableTerm(bus, VariableType.BUS_V, variableSet, bus.getV().eval());
equationSystem.createEquation(bus.getNum(), EquationType.BUS_V).addTerm(vTerm);
bus.setV(vTerm);
bus.getDiscreteVoltageControl()
Copy link
Member

Choose a reason for hiding this comment

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

You impress me here!

return bus.getDiscreteVoltageControl().filter(dvc -> bus.isDiscreteVoltageControlled())
.map(DiscreteVoltageControl::getTargetValue)
.orElse(getVoltageControlledTargetValue(bus)
.orElse(Double.NaN));
Copy link
Member

Choose a reason for hiding this comment

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

Indentation issue here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, but it's a bit unclear, so I changed it to

            .orElse(
                getVoltageControlledTargetValue(bus).orElse(Double.NaN)
            );

flo-dup and others added 2 commits June 16, 2021 15:07
Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>
@flo-dup flo-dup requested a review from annetill June 16, 2021 18:38
Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

91.9% 91.9% Coverage
0.0% 0.0% Duplication

@flo-dup flo-dup merged commit 56ff5da into master Jun 17, 2021
@flo-dup flo-dup deleted the discrete-control-optional branch June 17, 2021 08:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants