Separate hot water and space heating demand profiles #390
Conversation
| @@ -188,7 +218,8 @@ def get_daily_heat_demand( | |||
|
|
|||
| def heat_function(t, parameters): | |||
There was a problem hiding this comment.
I prefer to avoid using inner functions unless protection from outside callers is absolutely necessary. The main reason is that you cannot write tests for these.
Consider bringing it outside and renaming to _heat_function to specify that it's an inner function.
Update zenodo source of gridded weather data
- Remove lat-lon renaming. - add processing profiles of entire configured year range. - subset weather data to reduce minimal workflow runtime. - improve variable names. - add type hints and docstrings. - move assumptions to top of script.
|
Sorry for the messy operations - fixing a github diff issue was a pain. |
| """ | ||
| celsius_clipped = temperature.clip(min=HOT_WATER_LOWER_BOUND_TEMP) | ||
|
|
||
| return parameters["m_w"] * celsius_clipped + parameters["b_w"] + parameters["D"] |
There was a problem hiding this comment.
Small improvement here would be to create variables with explicit names for these parameters.
But it seems like when2heat also has this style, so it's a nice to have.
There was a problem hiding this comment.
Yeah, pretty sure they don't even have full names in the base source (the german govt. agency documentation).
| ) | ||
| for building in buildings | ||
| ], | ||
| dim=pd.Index(buildings, name="building"), | ||
| ) | ||
|
|
||
|
|
||
| def group_gridcells(gridded_data: xr.DataArray, weight: xr.DataArray) -> xr.DataArray: |
There was a problem hiding this comment.
Looks like an internal "helper" function. Consider _group_gridcells.
| The profiles need to be scaled so their magnitude matches annual heat demand data in a subsequent step. | ||
|
|
||
| Args: | ||
| path_to_population (str): Gridded population data, which will act as the weighting to got from grid-level profiles to profiles per model unit. |
There was a problem hiding this comment.
FYI: In other workflows, I stopped handing over paths as strings but rather handing data in and out. After doing this for a while, I'd recommend it. Maybe we could discuss this technical detail at one of the dev calls.
The function signature is then:
def business_logic(data: pd.DataFrame) -> pd.DataFrame:
# code here
return df
if __name__ == "__main__":
data = business_logic(pd.read_csv(snakemake.input[0]))
data.to_csv(snakemake.outptu[0])There was a problem hiding this comment.
I can see merits of both. Especially if reading the file is simple (no need to define header/row levels or parse dates, etc.), it would probably be easier to debug with data being handed in, rather than strings. Once you have lots going on with the data loading, I'm less certain about having so much going on in if __name__ == "__main__" because then debugging becomes hard (e.g. you can't load those parts of the script as methods in an interactive session).
Let's discuss at the next dev call.
Fixes #389
Checklist
Any checks which are not relevant to the PR can be pre-checked by the PR creator. All others should be checked by the reviewer. You can add extra checklist items here if required by the PR.