Skip to content

Feature/minimum degree topology#440

Merged
Jerry-Jinfeng-Guo merged 70 commits into
mainfrom
feature/minimum-degree-topology
Jun 19, 2024
Merged

Feature/minimum degree topology#440
Jerry-Jinfeng-Guo merged 70 commits into
mainfrom
feature/minimum-degree-topology

Conversation

@mgovers
Copy link
Copy Markdown
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
Comment thread power_grid_model_c/power_grid_model/include/power_grid_model/topology.hpp Outdated
Comment thread power_grid_model_c/power_grid_model/include/power_grid_model/sparse_ordening.hpp Outdated
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>
Comment thread power_grid_model_c/power_grid_model/include/power_grid_model/sparse_ordening.hpp Outdated
Comment thread power_grid_model_c/power_grid_model/include/power_grid_model/sparse_ordening.hpp Outdated
Comment thread power_grid_model_c/power_grid_model/include/power_grid_model/sparse_ordening.hpp Outdated
Comment thread power_grid_model_c/power_grid_model/include/power_grid_model/sparse_ordening.hpp Outdated
Comment thread power_grid_model_c/power_grid_model/include/power_grid_model/sparse_ordening.hpp Outdated
Comment thread power_grid_model_c/power_grid_model/include/power_grid_model/sparse_ordening.hpp Outdated
Comment thread power_grid_model_c/power_grid_model/include/power_grid_model/sparse_ordening.hpp Outdated
Comment thread power_grid_model_c/power_grid_model/include/power_grid_model/sparse_ordening.hpp Outdated
Comment thread power_grid_model_c/power_grid_model/include/power_grid_model/sparse_ordening.hpp Outdated
Comment thread power_grid_model_c/power_grid_model/include/power_grid_model/sparse_ordening.hpp Outdated
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Member Author

mgovers commented Jun 19, 2024

FINALLY
image
image

Copy link
Copy Markdown
Member

@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>
@sonarqubecloud
Copy link
Copy Markdown

@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
@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