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

Region coordinator #104

Closed
wants to merge 19 commits into from
Closed

Region coordinator #104

wants to merge 19 commits into from

Conversation

ckittl
Copy link
Member

@ckittl ckittl commented Dec 8, 2021

Resolves #91

@ckittl ckittl added the enhancement New feature or request label Dec 8, 2021
@ckittl ckittl added this to the Version 1.0 milestone Dec 8, 2021
@ckittl ckittl self-assigned this Dec 8, 2021
@codecov
Copy link

codecov bot commented Dec 8, 2021

Codecov Report

Merging #104 (57762ba) into dev (f5140d1) will not change coverage.
The diff coverage is n/a.

❗ Current head 57762ba differs from pull request most recent head 1bcd7c5. Consider uploading reports for the commit 1bcd7c5 to get more accurate results
Impacted file tree graph

@@          Coverage Diff          @@
##             dev    #104   +/-   ##
=====================================
  Coverage   0.00%   0.00%           
=====================================
  Files         25      25           
  Lines       1512    1512           
  Branches     236     236           
=====================================
  Misses      1512    1512           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 94afd57...1bcd7c5. Read the comment docs.

@ckittl
Copy link
Member Author

ckittl commented Dec 9, 2021

Currently, the test for area calculation is failing, because the area is overestimated by around 60 % as of test case. However, the issues seems to stem from the transformation from angles to metres / distances, as the circumfence of the building is also overestimated by ~ 30 %. I'd guess, that the root lays in the haversine equation or the assumed earth radius or whatever. We have to 🤿 into this.

sebastian-peter added a commit that referenced this pull request Jan 3, 2022
@ckittl
Copy link
Member Author

ckittl commented Jan 4, 2022

Currently, the test for area calculation is failing, because the area is overestimated by around 60 % as of test case. However, the issues seems to stem from the transformation from angles to metres / distances, as the circumfence of the building is also overestimated by ~ 30 %. I'd guess, that the root lays in the haversine equation or the assumed earth radius or whatever. We have to diving_mask into this.

With the changes of @t-ober in #175, the area calculation will be done in PowerSystemUtils package. Wait until that PR is closed and see, if the changed logic resolves the issue.

... as this will be part of PowerSystemUtils package
to restrict load clustering to happen inside land uses or without respecting them
of load locations, if that is needed.
what has to be done next.
@ckittl
Copy link
Member Author

ckittl commented Jan 5, 2022

According to the new region partitioning scheme proposed in #120, this implementation doesn't make sense anymore. Will provide the logic in another PR.

Please do NOT delete the branche. Parts of the code could still be useful.

@ckittl ckittl closed this Jan 5, 2022
@sebastian-peter sebastian-peter deleted the ck/#91-regionGenerator branch March 18, 2024 11:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement LvRegionGenerator
1 participant