-
Notifications
You must be signed in to change notification settings - Fork 14
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
384 bug voltage drop should be considered when selecting cable to connect new components #411
384 bug voltage drop should be considered when selecting cable to connect new components #411
Conversation
@@ -50,6 +50,11 @@ upper_limit_voltage_level_6 = 0.2 | |||
upper_limit_voltage_level_5 = 5.5 | |||
upper_limit_voltage_level_4 = 20.0 | |||
|
|||
lv_max_voltage_deviation = 0.03 | |||
# from VDE-AR-N 4100 (VDE-AR-N 4100) Anwendungsregel: 2019-04 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please add the comment above the value it applies to?
edisgo/tools/tools.py
Outdated
s_max : float or array-like | ||
Apparent power the cable must carry in MVA. | ||
r_total : float or array-like | ||
Total resistance in Ohms. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add "of the line" here, as well as for x_total and v_nom.
edisgo/tools/tools.py
Outdated
sin_phi = np.sqrt(1 - cos_phi**2) | ||
# Calculate the voltage difference using the formula from VDE-AR-N 4105 | ||
voltage_diff = np.abs( | ||
(s_max * 1e6 / (v_nom * 1e3)) * (r_total * cos_phi + sign * x_total * sin_phi) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please avoid multiplying and dividing unnecessarily due to rounding errors. 1e6/1e3/1e3 all cancels out, so it is not necessary here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, please don't return the absolute value here because it would be better to test, whether the voltage rises or drops.
edisgo/tools/tools.py
Outdated
length : float | ||
Length of the cable in km. Default: 0. | ||
max_voltage_diff : float | ||
Maximum voltage difference in pu. Default: None. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please elaborate this a bit more. It is the maximum allowed voltage difference over the line that connects the new component to the grid. If it is not specified, it is taken from the config files (section grid_connection, parameters lv_max_voltage_deviation and mv_max_voltage_deviation).
return voltage_difference_pu | ||
|
||
|
||
def select_cable( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please adapt the docstring of this function. The cable is not only selected based on the apparent power anymore but also considering the voltage deviation over the cable. The docstring should always start with a short one line description and then in the next paragraph it can be elaborated if needed.
edisgo/tools/tools.py
Outdated
length: float = 0, | ||
max_voltage_diff: float | None = None, | ||
max_cables: int = 7, | ||
cos_phi: float | None = 0.95, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please rename this parameter to power_factor to have it consistent with the rest of the code base. Also, I would prefer that there is no default value but that the default value is obtained from the configs. For this, the type of the component to be connected needs to be specified as input parameter.
edisgo/tools/tools.py
Outdated
max_voltage_diff: float | None = None, | ||
max_cables: int = 7, | ||
cos_phi: float | None = 0.95, | ||
inductive_reactance: bool = True, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use "reactive_power_mode" instead of "inductive_reactance" to have it consistent with the rest of the code base, e.g. here). Also, whether the reactive power is positive or negative in case of inductive behavior depends on the sign convention used. In eDisGo we use the generator sign convention for generators and storage units and the load sign convention for loads. This also needs to be fixed in the other functions.
edisgo/tools/tools.py
Outdated
cable_count = 1 | ||
|
||
if not cos_phi: | ||
cos_phi = 0.95 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not necessary because 0.95 is already the default value in the function head.
…of kilovolts (kV)
…of kilovolts (kV)
…tor as necessary inputs
…line type and component type as inputs
…nd suitable cable
Description
Add function to validate voltage drop against maximum allowable limits in cable selection
Fixes #384
Type of change
Please delete options that are not relevant.
New feature (non-breaking change which adds functionality)Breaking change (fix or feature that would cause existing functionality to not work as expected)Checklist:
pre-commit
hooksI have made corresponding changes to the documentation