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

Synchronization between views for v and angle bus values. #452

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

FranckLecuyer
Copy link
Contributor

@FranckLecuyer FranckLecuyer commented Sep 16, 2024

Please check if the PR fulfills these requirements

  • 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?

What kind of change does this PR introduce?

Bug fix

What is the current behavior?

Currently, when updating the V and angle value in Bus for different topologies (NodeBreaker or BusBreaker), the synchronization between CalculatedBus and ConfiguredBus is not consistently handled across views, leading to potential inconsistencies in voltage values.

What is the new behavior (if this is a feature change)?

  • When setV is called on getBus() from getBusBreakerView() in a NodeBreaker topology (which by default updates calculatedBusForBusBreakerView), calculatedBusForBusView should also be updated if it is not null.

  • When setV is called on getBus() from getBusView() in a NodeBreaker topology (which by default updates calculatedBusForBusView), calculatedBusesForBusBreakerView should also be updated if it is not null.

  • When setV is called on getBus() from getBusView() in a BusBreaker topology (which by default updates calculatedBusForBusView), configuredBus should also be updated.

  • When setV is called on getBus() from getBusBreakerView() in a BusBreaker topology (which by default updates configuredBus), calculatedBusForBusView should also be updated if it is not null.

And when calculating a new view, the v and theta values should be taken from the other source (NodeBreaker: busview/busbreakerview; BusBreaker: busview/configuredbus).

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

  • Yes
  • No

If yes, please check if the following requirements are fulfilled

  • The Breaking Change or Deprecated label has been added
  • The migration steps are described in the following section

What changes might users need to make in their application due to this PR? (migration steps)

Other information:

@SlimaneAmar
Copy link
Contributor

Resolve Sonar issue ?

}

private void updateConfiguredBuses(double newValue,
CalculatedBusAttributes calculatedBusAttributesBus,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
CalculatedBusAttributes calculatedBusAttributesBus,
CalculatedBusAttributes calculatedBusAttributes,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

if (CollectionUtils.isNotEmpty(calculatedBusAttributesList)) {
List<Integer> calculatedBusesNum = getAllTerminals().stream()
.filter(Terminal::isConnected)
.map(t -> ((CalculatedBus) t.getBusView().getBus()).getCalculatedBusNum()).distinct().toList();
Copy link
Collaborator

Choose a reason for hiding this comment

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

all terminals in one configured bus always map to the same busviewbus no ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right.
We consider now only the first bus in bus view ...

.map(Vertex::getBus)
.toList();

List<Bus> buses = index.getConfiguredBuses().stream()
Copy link
Collaborator

Choose a reason for hiding this comment

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

all configured bus of the network ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right.
We use now only the buses in voltage level bus breaker view ...

l1.getTerminal1().getBusView().getBus().setV(222);

// Verify the voltage update in BusBreakerView
assertEquals("Voltage should match in BusView after update in BusBreakerView", 222, l1.getTerminal1().getBusBreakerView().getBus().getV(), 0.0);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
assertEquals("Voltage should match in BusView after update in BusBreakerView", 222, l1.getTerminal1().getBusBreakerView().getBus().getV(), 0.0);
assertEquals("Voltage should match in BusBreakerView after update in BusView", 222, l1.getTerminal1().getBusBreakerView().getBus().getV(), 0.0);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

FranckLecuyer and others added 3 commits September 24, 2024 14:46
Add new test with multiple buses in bus view and bus breaker view in a voltage level

Signed-off-by: Franck LECUYER <[email protected]>
Signed-off-by: Franck LECUYER <[email protected]>
Copy link

sonarcloud bot commented Sep 25, 2024

@FranckLecuyer
Copy link
Contributor Author

Resolve Sonar issue ?

Removed

List<ConnectedSetResult<T>> connectedSetList = findConnectedSetList(index, voltageLevelResource, isBusView);
AtomicDouble v = new AtomicDouble(Double.NaN);
AtomicDouble angle = new AtomicDouble(Double.NaN);
Copy link
Collaborator

Choose a reason for hiding this comment

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

v set in all remaining buses

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah I see this bug made the AbstractMainConnectedComponentWithSwitchTest work because there is a bus without any vertex which would be unset, but since it's setting v in all remaining buses it sets it anyway.

This is all to do with the fact that we can not use the equipment Vertex (=non switch equipment) to match buses, we need to use the topology vertex (nodes/configuredbus)

String attributeName,
ToDoubleFunction<Bus> getValue,
ObjDoubleConsumer<ConfiguredBusAttributes> setValue) {
List<String> busesIds = calculatedBusAttributes.getVertices().stream()
Copy link
Collaborator

Choose a reason for hiding this comment

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

doesn't contain configuredbus without any vertex (equipment)

Comment on lines +203 to +209
List<String> busesIds = calculatedBusAttributes.getVertices().stream()
.map(Vertex::getBus)
.toList();

List<Bus> buses = getVoltageLevel().getBusBreakerView().getBusStream()
.filter(bus -> busesIds.contains(bus.getId()) && !Objects.equals(getValue.applyAsDouble(bus), newValue))
.toList();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
List<String> busesIds = calculatedBusAttributes.getVertices().stream()
.map(Vertex::getBus)
.toList();
List<Bus> buses = getVoltageLevel().getBusBreakerView().getBusStream()
.filter(bus -> busesIds.contains(bus.getId()) && !Objects.equals(getValue.applyAsDouble(bus), newValue))
.toList();
List<Bus> buses = ((VoltageLevelImpl) getVoltageLevel()).getResource().getAttributes()
.getBusToCalculatedBusForBusView().entrySet().stream()
.filter(entry -> getCalculatedBusNum() == entry.getValue())
.map(Map.Entry::getKey)
.map(getVoltageLevel().getBusBreakerView()::getBus)
.toList();

something like that ?

Signed-off-by: HARPER Jon <[email protected]>
@jonenst jonenst mentioned this pull request Sep 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants