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

Feature/minimum degree topology #440

Merged
merged 70 commits into from
Jun 19, 2024

Conversation

mgovers
Copy link
Member

@mgovers mgovers commented Nov 27, 2023

Fixes #433

Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
@mgovers mgovers added the improvement Improvement on internal implementation label Nov 27, 2023
@mgovers mgovers self-assigned this Nov 27, 2023
mgovers and others added 9 commits December 1, 2023 15:40
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
…e-topology

Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
mgovers and others added 7 commits January 2, 2024 15:59
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
…e-topology

Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
@mgovers mgovers changed the base branch from main to feature/auto-tap-validation-test May 24, 2024 08:50
@mgovers mgovers changed the base branch from feature/auto-tap-validation-test to main May 24, 2024 08:50
@mgovers
Copy link
Member Author

mgovers commented Jun 11, 2024

Cfr. our discussions, we run the benchmark case and if the amount of fill-ins is the same for both implementations, it's ready for merge.

The amount of fill-ins differs on the benchmark case: for the meshed grid cases, all runs return 91 fill-ins for the current main and 98 fill-ins for this branch, which is a regression.

Further investigation needed

@mgovers
Copy link
Member Author

mgovers commented Jun 17, 2024

cfr. discussion: boost and this branch use slightly different choices regarding the chosen point, but they scale similarly:

  • sometimes, boost is better (e.g. benchmark case for large grids),
  • at other times, this branch provides less fill-ins (e.g. benchmark case for small grids)
  • they both are not necessarily optimal solutions

An efficient exact optimal minimum degree algorithm is proposed in https://epubs.siam.org/doi/epdf/10.1137/1.9781611976465.45 that could provide speed-ups.
Simpler solutions also exist, like randomly trying n vertices with the same minimal degree instead of only a single arbitrary vertex of minimum degree (e.g. the first, which is the one implemented in this branch)

For now, this solution seems to be OK and it is ready for review and merge as is.

@mgovers mgovers marked this pull request as ready for review June 17, 2024 12:17
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
mgovers added 6 commits June 17, 2024 15:22
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
@mgovers
Copy link
Member Author

mgovers commented Jun 19, 2024

FINALLY
image
image

Copy link
Contributor

@Jerry-Jinfeng-Guo Jerry-Jinfeng-Guo left a comment

Choose a reason for hiding this comment

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

I will approve the PR once ci is fixed and code cleaned

mgovers added 3 commits June 19, 2024 14:51
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
Copy link

@Jerry-Jinfeng-Guo Jerry-Jinfeng-Guo added this pull request to the merge queue Jun 19, 2024
Merged via the queue into main with commit e2652ff Jun 19, 2024
26 checks passed
@Jerry-Jinfeng-Guo Jerry-Jinfeng-Guo deleted the feature/minimum-degree-topology branch June 19, 2024 15:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improvement on internal implementation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE] In-house minimum degree reordering
3 participants