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.

Sign up for GitHub

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

Jump to bottom

Fix capacity factor timeseries in convert.py #346

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
+322 −118
Open

Fix capacity factor timeseries in convert.py #346

Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4b2e05f
Fix capacity factor timeseries in convert.py
TimFuermann Apr 12, 2024
2c897e7
Merge branch 'PyPSA:master' into master
TimFuermann Nov 20, 2024
2184a27
Updated convert_and_aggregate
TimFuermann Nov 20, 2024
450f63a
Update test_preparation_and_conversion.py
TimFuermann Nov 20, 2024
9f0f9a8
Add version to __init__.py
TimFuermann Nov 26, 2024
024a3c3
Merge branch 'PyPSA:master' into master
TimFuermann Dec 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 18 additions & 13 deletions atlite/convert.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,8 @@ def convert_and_aggregate(
resource : xr.DataArray
Time-series of renewable generation aggregated to buses, if
`matrix` or equivalents are provided else the total sum of
generated energy.
generated energy. If capacity_factor flag is set the capacity
factor for each grid cell is returned instead.
units : xr.DataArray (optional)
The installed units per bus in MW corresponding to `layout`
(only if `return_capacity` is True).
Expand All @@ -128,22 +129,24 @@ def convert_and_aggregate(

no_args = all(v is None for v in [layout, shapes, matrix])

results = None

if no_args:
if per_unit or return_capacity:
raise ValueError(
"One of `matrix`, `shapes` and `layout` must be "
"given for `per_unit` or `return_capacity`"
)
if capacity_factor or capacity_factor_timeseries:
if capacity_factor_timeseries:
res = da.rename("capacity factor")
else:
res = da.mean("time").rename("capacity factor")
res.attrs["units"] = "p.u."
return maybe_progressbar(res, show_progress, **dask_kwargs)

if capacity_factor or capacity_factor_timeseries:
if capacity_factor_timeseries:
results = da.rename("capacity factor")
else:
res = da.sum("time", keep_attrs=True)
return maybe_progressbar(res, show_progress, **dask_kwargs)
results = da.mean("time").rename("capacity factor")
results.attrs["units"] = "p.u."

else:
results = da.sum("time", keep_attrs=True)
Copy link
Member

Choose a reason for hiding this comment

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

Removing the return means that we no longer end the function here, which leads to undefined issues later. I know, this one is open for quite a while now, but could you have another look?

Otherwise this function is quite a mess and could get a general refactoring with your proposed changes included

Copy link
Author

Choose a reason for hiding this comment

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

@lkstrp: I need to check it locally, but could you explain which undefined issues will occur later? I know the fix is not ideal, and was actually just done that kind of messy to not restructure the whole function.

Copy link
Member

Choose a reason for hiding this comment

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

Further down, matrix is not defined. If you merge the current master, this will hopefully show up in the tests. They may not run through because of a missing cdsapi token, as this branch was derived earlier. But you can run them locally in any case using pytest ..

if index is None:
    index = pd.RangeIndex(matrix.shape[0])

But as you said, restructuring the whole function is better. If you have already looked at it, feel free. Otherwise I will deal with it at some point.

Copy link
Author

Choose a reason for hiding this comment

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

@lkstrp I restructured the function and tested it. I commit it soon. Nevertheless, this would need to change some things in the origional test files, too and in the usage. I explain it with the commit, Best, Tim

Copy link
Member

Choose a reason for hiding this comment

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

Sounds great! Thank you @TimFuermann

Copy link
Author

@TimFuermann TimFuermann Nov 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@lkstrp Just uploaded a new version of how the convert and aggregate could be used more consistently. Nevertheless this includes heavy changes in the input flags, but allows for a more consistent way of using them. A default capacity layout is used if no capacities are provided. This is a constant 1MW per gridcell (which should be the same as looking for capacity factors). With this new implementation, capacity factors for regions, per gridcell, aswell as aggregated can be calculated.

I furthermore added minor changes to the pytest test_preparation_and_conversion, since the output format slightly changes now, as well as the way how the input flags need to be set.


if matrix is not None:
if shapes is not None:
Expand Down Expand Up @@ -187,16 +190,18 @@ def convert_and_aggregate(
if index is None:
index = pd.RangeIndex(matrix.shape[0])

results = aggregate_matrix(da, matrix=matrix, index=index)
if results is None:
results = aggregate_matrix(da, matrix=matrix, index=index)

if per_unit or return_capacity:
caps = matrix.sum(-1)
capacity = xr.DataArray(np.asarray(caps).flatten(), [index])
capacity.attrs["units"] = "MW"

if per_unit:
results = (results / capacity.where(capacity != 0)).fillna(0.0)
results.attrs["units"] = "p.u."
if not (capacity_factor or capacity_factor_timeseries):
results = (results / capacity.where(capacity != 0)).fillna(0.0)
results.attrs["units"] = "p.u."
else:
results.attrs["units"] = "MW"

Expand Down