-
Notifications
You must be signed in to change notification settings - Fork 9
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
Add_bus_gen #59
Add_bus_gen #59
Conversation
I've got a problem with the add_electricity rule. When I check the values in the renewable generator, the values of p_set and p_max_pu are NaN. Can you help me understand why?@davide-f |
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
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.
Great @Margherita-Capitani :D Adding few comments :)
In the terminal, I'd recommend to install the precommit by typing:
conda activate pypsa-earth
pre-commit install
Moreover, ensure in vscode to have pypsa-earth as default python environment accessible from view->command pallette->Python: Select interpreter -> Python environments -> pypsa-earth
scripts/create_network.py
Outdated
) | ||
|
||
# calculate_power_node_position( |
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.
No more needed?
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.
mmm no I can delete it from here, just define the function for calculating the position of the generating nodes, which is then used directly in the create_microgrid_network function
scripts/add_electricity.py
Outdated
microgrid_ids = [f"microgrid_{i+1}" for i in range(len(number_microgrids))] | ||
for microgrid in microgrid_ids: | ||
ppl.index = ppl.index + "_" + microgrid # TODO: review this | ||
ppl["bus"] = microgrid + "_gen_bus" |
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 in theory should not be needed. In custom_powerplants, the goal is to allow users to optionally add custom powerplants and they should specify the inputs that are also translated into the bus where it is installed.
It would be great to add also generators to microgrid buses when extendable, similarly to the renewable sources, but we can keep this for another PR.
In this PR, just preserving the previous behaviour is fine.
To do so, probably, there may be the need to change the reference values in custom powerplants to match the new bus names probably
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.
Perfect I got it, restore to the previous version!
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.
Many thanks, impact is growing :D
Functionality wise we are almost there, just some minor improvements :)
scripts/create_network.py
Outdated
gdf = cluster_bus | ||
data = [] | ||
for _, feature in gdf.iterrows(): | ||
cluster = feature["cluster"] | ||
coordinates = feature.geometry.coords[0] | ||
data.append({"cluster": cluster, "x": coordinates[0], "y": coordinates[1]}) | ||
# Create a DataFrame | ||
df = pd.DataFrame(data) | ||
# Add load_sums as a new column to the DataFrame | ||
df["cluster_load"] = load_sums.values | ||
weights = [] | ||
for i in load_sums.values: | ||
weight = i / sum(load_sums.values) | ||
weights.append(weight) | ||
df["weight"] = weights | ||
x = 0 | ||
y = 0 | ||
for i in range(len(df)): | ||
x += df.loc[i, "x"] * df.loc[i, "weight"] | ||
y += df.loc[i, "y"] * df.loc[i, "weight"] |
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 generally slow; executing for loops explicitly on dataframes should be avoided when possible.
For example, to access the x variable of geometry, we can do:
cluster_bus.geometry.x
ideally, if cluster_bus and load_sums share the same index, we could do:
gdf = cluster_bus.set_index("cluster")
x_wgt_avg = gdf.geometry.x * load_sums / load_sums.sum()
``` [This assuming load_sums is a pandas series, otherwise there is the need to select the specific column
the same can be done for y.
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.
super i fix it in this way!
scripts/build_demand.py
Outdated
# Verify that the length of snapshots matches the length of the load data | ||
if len(snapshots_range) == len(all_load_per_cluster): | ||
all_load_per_cluster.insert(0, "timestamp", snapshots_range) |
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 explain this?
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.
was a remnant of a correction of an error I was experiencing, it should actually be lighter and work even so!
for more information, see https://pre-commit.ci
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.
Just few comments and ready to go after you check them :)
config.distribution.yaml
Outdated
# 1 = an average hourly profile is calculated by exploiting the ramp tool | ||
# 2 = an average hourly profile and its standard deviation is calculated using the ramp tool, | ||
# and both quantities are used to calculate demand. | ||
# 0 = a predetermined hourly profile is used | ||
# 1 = an average hourly profile is calculated by exploiting the ramp tool when std on also the | ||
# standard deviation is calculated and used to calculate demand. |
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.
We can keep it this way; we can also replace the number coding with a string code, e.g. "from_file"/"ramp" etc.
It is not necessary for this PR unless you prefer it
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.
Fix it!! :)
max_hours=max_hours["battery"], # Lead_acid and lithium have the same value | ||
cyclic_state_of_charge=True, | ||
) | ||
for microgrid in microgrid_ids: |
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.
In the future we can revise this and avoid the for loop
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.
PerfectI keep it in mind!
With this PR, a bus is added to which the entire generation can be connected. The bus is positioned so as to minimise the distance to the most energy-intensive network nodes.