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

Include missing parameters for new lines #123

Merged
merged 2 commits into from
Oct 2, 2023

Conversation

CarlosEpia
Copy link

@CarlosEpia CarlosEpia commented Sep 14, 2023

Fixes #120.
Already tested on the server.

@CarlosEpia CarlosEpia self-assigned this Sep 14, 2023
@CarlosEpia CarlosEpia marked this pull request as ready for review September 26, 2023 14:27
Copy link
Member

@ulfmueller ulfmueller left a comment

Choose a reason for hiding this comment

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

It is probably all fine and it is rather a matter of taste... I would either change the comment of the lines of r_per_km, b_per_km, cost_km or change to calculation to a more generic formula based version.

@@ -113,15 +118,26 @@ def add_line(x0, y0, x1, y1, v_nom, scn_name, cables):
if v_nom == 220:
s_nom = 520
x_per_km = 0.001 * 2 * np.pi * 50
r_per_km = 0.097 # based on average r from similar lines
Copy link
Member

Choose a reason for hiding this comment

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

I would prefer a more generic calculation like it is done for the other lines in osmTGmod and like it is done for x (line above). So, see osmTGmod and there. Same thing for all the following r and b calculations

Copy link
Member

Choose a reason for hiding this comment

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

But actually if it is a length-specific value and only for a 220kV overhead line, all of the lines of this type should have the same value, right? So if you take average should be the same as taking any value of this group? So value-wise it should not make a difference, I guess. It just reads a bit like as it would be just a rough estimate...

Copy link
Author

Choose a reason for hiding this comment

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

I also thought that all the lines with the same capacity and voltage level should have the same resistance/km but it is not the case. I will proceed to write a more generic calculation using the values here.

@@ -113,15 +118,26 @@ def add_line(x0, y0, x1, y1, v_nom, scn_name, cables):
if v_nom == 220:
s_nom = 520
x_per_km = 0.001 * 2 * np.pi * 50
r_per_km = 0.097 # based on average r from similar lines
b_per_km = 3.066e-6 # based on average b from similar lines
cost_km = 40.697 # based on average costs from similar lines
Copy link
Member

Choose a reason for hiding this comment

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

I am not quite sure where it is normally set. But also here should be somewhere a value or calculation which could be directly used.

Copy link
Author

Choose a reason for hiding this comment

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

Change applied.

@CarlosEpia
Copy link
Author

@ulfmueller I think now it is worth checking this PR again.

Copy link
Member

@ulfmueller ulfmueller left a comment

Choose a reason for hiding this comment

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

I like the changes!

@CarlosEpia CarlosEpia merged commit 7fc4fb1 into dev Oct 2, 2023
3 checks passed
@CarlosEpia CarlosEpia deleted the fixes/#120-lines-without-impedance branch November 22, 2023 14:04
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.

lines with resistance = 0
2 participants