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

Fix missing shape #405

Merged
merged 5 commits into from
Jul 21, 2022
Merged

Fix missing shape #405

merged 5 commits into from
Jul 21, 2022

Conversation

flo-dup
Copy link
Contributor

@flo-dup flo-dup commented Jul 11, 2022

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

What kind of change does this PR introduce?
Bug fix

What is the current behavior?
Shape missing on specific intern cells, leading to erroneous diagrams:
screenshot

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

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

@flo-dup
Copy link
Contributor Author

flo-dup commented Jul 11, 2022

In InternCell, if candidateLegs.size() == 2 but getBusNodes().size() != 2, the shape remained UNDEFINED, and it was not treated subsequently in Subsection::identifyVerticalInternCells method, because it skipped the intern cells with that kind of shape.

flo-dup added 3 commits July 11, 2022 18:27
Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>
vertical detection done also on UNDEFINED shapes in Subsection

Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>
Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>
@flo-dup flo-dup force-pushed the fix_missing_crossover branch from ed2f5e2 to bfafb77 Compare July 11, 2022 16:28
@flo-dup flo-dup requested a review from BenoitJeanson July 12, 2022 08:03
Copy link
Contributor

@BenoitJeanson BenoitJeanson left a comment

Choose a reason for hiding this comment

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

one remark to check before merging.
Smart changes!

// if more than one bus on one side, the intern cell is
// - either crossover (two sections): detected later in Subsection::identifyCrossOverAndCheckOrientation
// - or vertical (one section): detected later in Subsection::identifyVerticalInternCells
if (candidateLegs.stream().map(LegBlock::getBusNodes).allMatch(bn -> bn.size() == 1)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

At first sight, an UNILEG could be tagged as MAYBEFLAT if it is connected to a single bus (this case is actually very weird), maybe you should perform the test on UNILEG first

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, indeed, good catch! But as this is unrelated to current issue and as it implies deeper changes, I'll create an issue about that.

Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>
@flo-dup flo-dup force-pushed the fix_missing_crossover branch from 389c77a to f228695 Compare July 20, 2022 14:28
Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

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

97.1% 97.1% Coverage
0.0% 0.0% Duplication

@flo-dup flo-dup merged commit d695b0c into main Jul 21, 2022
@flo-dup flo-dup deleted the fix_missing_crossover branch July 21, 2022 14:56
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