Add VDI profiles based on lpagg #50

uvchik · 2022-04-12T07:40:18Z

The basic code is taken from https://github.com/jnettels/lpagg but most of the code has been revised for modular use and to speed up the code. The lpagg package contains various functionality but only the VDI-profiles and reading the TRY data from DWD is used.

To test the new profiles use the vdi_profile_example in the examples section. The parameter of the houses are added within a loop. I used this to multiply the number of houses to check how long it would take to model e.g. 1000 houses. 1000 houses on an hourly base will take less than a minute, which shows that the draft is at least fast enough 🏎️

Related to #36

Install this branch to test the code:

pip install https://github.com/oemof/demandlib/archive/features/add-vdi-from-lpagg

It is the first draft, I am open for concrete ideas.

add tests
add documentation
clean code

jnettels · 2022-04-13T11:41:38Z

@uvchik thanks very much, especially the speed looks promising! Not iterating through rows does indeed help a lot :-)
I was able to run your example. I have reduced it to a calculation for a single house, and I am still getting differences between my implementation and yours, even if they are very small. It comes down to me identifying e.g. 21. October 2017 in TRY04 as WWB, while your code yields WWH. The difference is the daily mean of the cloud cover (< or >5). This only happens for 4 days, I think.

I am not 100% sure yet, but I think it comes down to the following: At 15min frequency, Pandas df.resample("D").mean() will use the values between (including) 2017-10-21 00:00 and 2017-10-21 23:15 to give the mean of 2017-10-21. However, in the weather data, 2017-10-21 00:00 describes the weather conditions between 2017-10-20 23:00 and 2017-10-21 00:00. The value in 2017-10-21 00:00 therefore belongs to 2017-10-20. To put it more simply: By DWD definition, the weather data does not start at 2017-1-1 00:00, but 2017-1-1 01:00.
You seem to ignore that and simply load the weather data, starting at 2017-1-1 00:00, which may actually solve those problems. It may very well be that your solution is correct, and my many workarounds lead to incorrect results. Whenever I think about this stuff, I have to double and tripple check.

Some more stuff that might need consideration:

You seem to allow leap years, but that would lead to missing rows in the weather data. Setting vdi.Region(year=2020) currently throws ValueError: Length mismatch: Expected 8760 rows, received array of length 8784 and a more helpful message might be appropriate
The defining temperature for summer should be exposed to the user, with default 15°C (Tamb_heat_limit in my code)
If no weather data is given, the default weather file for the given TRY region should be loaded
The user should be able to override the used weather timeseries. (These could then be used for leap years, if desired.)
While using vdi.Region(holidays=holidays.country_holidays('DE', subdiv='NI')) from https://pypi.org/project/holidays/ already works (yay!), it could be actively encouraged in vdi_profile_example. I think that project is awesome.

There might be more stuff, but these are all minor issues I think. I need some time for more investigation, though, to see if the API fits all the needs that I can come up with.

uvchik · 2022-04-13T19:20:24Z

The weather data seems to be fine to me now. The DWD data starts with 01-01-01 (Month-Day-Hour), while weather data is typically right stamped. So in my left stamped index the first value is 2017-01-01 00:00. For me a right stamped index seems to be counter intuitive for a daily index
My idea is to duplicate the nearest day with the same weekday to create the typical day for the 29th of February. If it is a Sunday I would use the last Sunday, if it is a weekday I would use the last weekday which would be the 28th or the 27th. I think this is good enough regarding all the other assumptions. (still a ToDo)
I am not sure whether we should offer this or not. Some of the factors are connected to the TRY region. I seems to be strange to use an arbitrary data set with this procedure. In that case one has to pass the weather data and the TRY region.
I use workalendar in my example, but feel free to use your favourite holiday package.

pep8speaks · 2022-04-13T20:35:12Z

Hello @uvchik! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2022-09-06 18:25:57 UTC

jnettels · 2022-04-27T15:43:46Z

Here are the notes about what we discussed:

Make summer_temperature_limit (currently set in the ini file) an attribute of each house. As a consequence, different series of typical days have to be created for each unique temperature. These typical days then have to be correctly used for the correct houses
Allow manipulation of the weather data. Using the default weather data should be the default behaviour, but using your own weather data should also be possible. For example, vdi.Region could have a function get_weather() which would yield a DataFrame which can be manipulated and a function set_weather() which overwrites that default weather. This can be used before calling get_load_curve_houses().

Some more notes that came up while replacing my own implementation in lpagg with yours:

I think it could be more convenient for further processing if the DataFrame lc returned by vdi.region.get_load_curve_houses() had named column levels. Simplest would be lc.columns.set_names(['name', 'house_type', 'energy'], inplace=True) before returning it, but it might me more appropriate to apply the names at different concat() stages, like the following. However, this probably should be in line with the results returned from the existing BDEW implementation.

            load_curve_house.columns = pd.MultiIndex.from_product(
                [
                    [house["house_type"]],
                    load_curve_house.columns,
                ],
                names=['house_type', 'energy']
            )
            house_profiles[house["name"]] = load_curve_house
        return pd.concat(
            house_profiles.values(), keys=house_profiles.keys(), axis=1,
            names=['name']
        )

Another very general point: Currently the new module is simply named vdi, which is extremely general. Would vdi4655 not be more appropriate, in case other vdi norms are implemented in the future?
If I am not mistaken, TRY (as an attribute of each house) is unused and should be removed in the example script (since it is defined by the region instead)
When I implemented it, I made the house data a nested dictionary. You changed it to a list of dictionaries instead. In retrospect, I think both are not ideal. I later added an option to lpagg to read the house data from a table file. (See this example xlsx: https://github.com/jnettels/lpagg/blob/master/lpagg/examples/VDI_4655_houses_example.xlsx). When you start to define large lists of houses, a table is in my opinion the most intuitive data type. Therefore I would like to consider changing the argument houses of vdi.Region() to a DataFrame. Of course, you could internally convert it to a list of dictionaries immediately and not change your code. I am only talking about the interface.
The point above would also provide an opportunity to implement an empty houses DataFrame that contains all the required rows. If creating an object of vdi.Region() without the houses argument, you could afterwards ask for the empty DataFrame and fill it with your data? This way the required attributes of each house are actually documented. ...or is that too convoluted?
If I see correctly, the house attributes N_Pers and N_WE are currently unused. In VDI4655, they serve two purposes:
-- Check validity: VDI only claims support for N_Pers <= 12 (for EFH) and N_WE <= 40 (for MFH)
-- Provide default values for Q_TWW_a and W_a (annual energy for domestic hot water and electricity)
If we are not using them here, they should drop from the house attributes. But I think we should use them. I have a separate function lpagg.VDI4655.get_annual_energy_demand() which takes the houses data, checks those requirements and fills in the default values if they were not provided by the user. After some clean-up, this could become an optional preprocessing step before putting houses into vdi.Region(). Or vdi.Region() does it internally, but that would be less transparent to the user.

(Please tell me if I should do some of those implementations myself, or if you prefer to do it yourself.)

Further discussion:

In your example script, the houses get the attributes copies and sigma. If I see correctly, these are not currently used and could therefore be deleted for now. But they are related to the concept of "generating simultaneity effects through time shifts based on random draws from a normal distribution". This could become a seperate issue and pull request, but I would like to discuss if this could have a place in demandlib as well

jnettels · 2022-04-29T13:38:53Z

src/demandlib/vdi/regions.py

+            load_curve_house.columns = pd.MultiIndex.from_product(
+                [
+                    [house["house_type"]],
+                    load_curve_house.columns,
+                ]
+            )
+            house_profiles[house["name"]] = load_curve_house
+        return pd.concat(
+            house_profiles.values(), keys=house_profiles.keys(), axis=1
+        )


My suggestion for named levels:

load_curve_house.columns = pd.MultiIndex.from_product( [ [house["house_type"]], load_curve_house.columns, ], names=['house_type', 'energy'] ) house_profiles[house["name"]] = load_curve_house return pd.concat( house_profiles.values(), keys=house_profiles.keys(), axis=1, names=['name'] )

p-snft

I see copyright issues because of the implementation of the standard:

Copyright:
Verein Deutscher Ingenieure e.V.
VDI Standards Department
VDI-Platz 1, 40468 Duesseldorf, Germany
Reproduced with the permission of the Verein Deutscher Ingenieure e.V.,
for non-commercial use only.

p-snft · 2022-05-02T08:36:20Z

examples/electricity_demand_example.py

@@ -27,7 +27,7 @@
 import demandlib.bdew as bdew
 import demandlib.particular_profiles as profiles

-# The following dictionary is create by "workalendar"
+# The following dictionary has been created by "workalendar"


Suggested change

# The following dictionary has been created by "workalendar"

# The following dictionary is created by "workalendar"

I am a bit puzzled here. First, you write that the dictionary has been/ is created using workalendar but later you define the holidays yourself.

While I am writing comments here... :-)
Since those lines are commented out, I think this is just to suggest how to use an alternative library, without creating a new dependency for the installation of demandlib. My only suggestion would be to also name https://pypi.org/project/holidays/ as another option. Personally, it took me much too long to stumble across it.

examples/vdi_profile_example.py

p-snft · 2022-05-02T08:49:13Z

examples/vdi_profile_example.py

+            "N_Pers": 3,
+            "name": "EFH_{0}".format(n),
+            "N_WE": 1,
+            "Q_Heiz_a": 6000,
+            "TRY": 4,
+            "copies": 24,
+            "house_type": "EFH",
+            "sigma": 4,
+            "Q_TWW_a": 1500,
+            "W_a": 5250,


Language: "EFH", "WE", "Heiz" and "TWW" are German, all others should work in English. (First, I misinterpreted WE as weekend.)

Comprehensibility: What does W_a mean?

This has bugged me, too. The reason is sticking to the notation in the norm itself. Even though there is an English version of VDI 4655, that one still uses the German notation. To name a few:
Q: Heat
W: Electrical work
TWW: Domestic hot water
TT: Typical day
a: Year
WE: Flats

Therefore W_a is the electrical work of one year in kWh.

While this may not be ideal, I'd still argue that sticking to the source notation is very important, at least for the internal code. Making the public API English is something that might be worth considering, as a compromise.

You are right, it might be better to use English abbreviations in the code.

src/demandlib/vdi/regions.py

jnettels · 2022-06-20T13:05:58Z

I took the liberty to make two changes based on my previous suggestions: Named column levels and the ability to set my own weather file path. These changes help me to use demandlib within lpagg.

I think now the most important remaining issue is the ability to define the summer_temperature_limit for each house. My other suggestions are just for convenience.

The temperature limit is connected to the efficiency status of the house.

jnettels · 2022-06-30T11:48:01Z

I have not tested it yet, but thanks for adding the temperature limits. Though, since you removed my solution to define a custom weather file, I need to ask: Was it rude (or just inconvenient) of me to push to this branch, or did you not agree with my approach?

p-snft

I like the verbose inline documentation explaining what the code does. There is just one location where I found a missing piece of information.

More general, I am still not sure about the usage of German abbreviations. It might or might not be advised. (As the English version of the norm is using them, it can at leats be justified.)

p-snft · 2022-06-30T11:48:10Z

src/demandlib/vdi/__init__.py

+from .dwd_try import find_try_region  # noqa: F401
+from .regions import Region  # noqa: F401


Suggested change

from .dwd_try import find_try_region # noqa: F401

from .regions import Region # noqa: F401

from .dwd_try import find_try_region

from .regions import Region

__all__ == [

"find_try_region",

"Region",

]

p-snft · 2022-06-30T11:51:15Z

src/demandlib/vdi/regions.py

+
+        Returns
+        -------
+


Missing documentation of the return value.

jnettels

I think we should use the default temperature limits if they are not defined by the user.
Also, I added my idea for changing the weather data as a suggestion.

src/demandlib/vdi/regions.py

jnettels · 2022-07-01T16:42:59Z

src/demandlib/vdi/regions.py

+                summer=house["summer_temperature_limit"],
+                winter=house["winter_temperature_limit"],


Suggested change

summer=house["summer_temperature_limit"],

winter=house["winter_temperature_limit"],

summer=house.get("summer_temperature_limit", 15),

winter=house.get("winter_temperature_limit", 5),

I don't know how to best improve this, but it feels wrong to define the default values at two places in the code...

jnettels · 2022-07-01T16:53:21Z

src/demandlib/vdi/regions.py

+        seasons=None,
+        holidays=None,
+        houses=None,
+        resample_rule=None,


I am also adding the code snippets for changing the weather file.

Suggested change

resample_rule=None,

resample_rule=None,

file_weather=None,

jnettels · 2022-07-01T16:53:26Z

src/demandlib/vdi/regions.py

+            dictionary need to have the following keys.
+        resample_rule : str
+            Time interval to resample the profile e.g. 1H (1 hour) or 15min.
+            The value will be passed to the pandas resample method.


Suggested change

The value will be passed to the pandas resample method.

The value will be passed to the pandas resample method.

file_weather : str (optional)

Path to a 'test reference year' (TRY) weather file by German DWD

(Deutscher Wetterdienst). If None, the file fitting the given

try_region will be loaded.

As long as you adhere to the TRY format, the actual file does not need to be provided by DWD.

jnettels · 2022-07-01T16:53:37Z

src/demandlib/vdi/regions.py

+        self._try_region = try_region
+
+        self._year = year
+        self.weather = None


Suggested change

self.weather = None

self.weather = None

self.file_weather = file_weather

jnettels · 2022-07-01T16:53:45Z

src/demandlib/vdi/regions.py

+        fn_weather = os.path.join(
+            os.path.dirname(__file__),
+            "resources_weather",
+            "TRY2010_{:02d}_Jahr.dat".format(self._try_region),
+        )
+        self.weather = dwd_try.read_dwd_weather_file(fn_weather)


Suggested change

fn_weather = os.path.join(

os.path.dirname(__file__),

"resources_weather",

"TRY2010_{:02d}_Jahr.dat".format(self._try_region),

)

self.weather = dwd_try.read_dwd_weather_file(fn_weather)

if self.file_weather is None:

self.file_weather = os.path.join(

os.path.dirname(__file__),

"resources_weather",

"TRY2010_{:02d}_Jahr.dat".format(self._try_region),

)

self.weather = dwd_try.read_dwd_weather_file(self.file_weather)

p-snft

There seems to be old code that has been commented out instead of being deleted.

p-snft · 2022-07-05T06:32:40Z

src/demandlib/vdi/regions.py

+            dictionary need to have the following keys.
+        resample_rule : str
+            Time interval to resample the profile e.g. 1H (1 hour) or 15min.
+            The value will be passed to the pandas resample method.


As long as you adhere to the TRY format, the actual file does not need to be provided by DWD.

p-snft · 2022-07-05T06:40:35Z

src/demandlib/vdi/regions.py

+        # typtage_combinations = settings["typtage_combinations"]
+        # houses_list = settings["houses_list_VDI"]


Is this old code that has been commented out instead of being deleted?

p-snft · 2022-07-05T06:40:56Z

src/demandlib/vdi/regions.py

+        # if settings.get("zero_summer_heat_demand", None) is not None:
+        #     # Reduze the value of 'F_Heiz_TT' to zero.
+        #     # For modern houses, this eliminates the heat demand in summer
+        #     energy_factors_df.loc[
+        #         (slice(None), slice(None), "F_Heiz_TT"), ("SWX", "SSX")
+        #     ] = 0


Is this old code that has been commented out instead of being deleted?

p-snft · 2022-07-05T06:41:15Z

src/demandlib/vdi/regions.py

+            # df = pd.concat([df_typ, mytime], axis=1).ffill()
+            # df.drop(df.head(1).index, inplace=True)


Is this old code that has been commented out instead of being deleted?

p-snft · 2022-07-05T06:42:48Z

src/demandlib/vdi/regions.py

+            # The typical day calculation inherently does not add up to the
+            # desired total energy demand of the full year. Here we fix that:
+            # houses_dict = {h["name"]: h for h in self.houses}


Is this old code that has been commented out instead of being deleted?

p-snft · 2022-07-05T07:13:29Z

As this is a draft, I marked it as such.

Side note: I really like the feature. It's very handy to have generation of time series for SH, DHW, and electricity consumption at hand.

Co-authored-by: Joris Zimmermann <[email protected]>

uvchik · 2022-07-12T19:48:38Z

For me it is a bit rude to write your code into my approach without any comment. I already wrote improvements and got dozen of merge conflicts. If I know somebody else is working on the code I would push more often to avoid such problems.

I do have a different approach for the weather file so I think the best way would be to finish this PR with cleaning the code and adding some tests and then start a new PR for the weather data. That makes it easier to track changes and to focus the discussion. One reason for that is that the original approach is connect to the TRY data. I am fine with adding your own weather data but it must be clear that you really should know what you are doing.

jnettels · 2022-07-14T13:26:27Z

Please accept my apologies for pushing into this branch, then. While I did leave a small comment about it here, I see how it creates unnecessary merge conflicts. Afterwards I learned how to use the "Suggested Change" feature and will stick to that in the future.
Doing a new PR for the weather data is fine with me, too.

p-snft · 2022-09-07T07:09:44Z

src/demandlib/vdi/dwd_try.py

+def read_dwd_weather_file(weather_file_path=None, try_region=None):
+    """Read and interpolate 'DWD Testreferenzjahr' files."""
+    if weather_file_path is None:
+        weather_file_path = os.path.join(
+            os.path.dirname(__file__),
+            "resources_weather",
+            "TRY2010_{:02d}_Jahr.dat".format(try_region),
+        )
    # The comments in DWD files before the header are not commented out.
    # Thus we have to search for the line with the header information:


Carefully reading the description, it is clear that we are talking about Testreferenzjahr files. However, there are a lot of types of DWD files (i.e. recordings from weather stations), maybe we could abbreviate it to TRY files like DWD does.

jnettels · 2023-06-17T20:46:49Z

Hi there,

it has been quite a while, but I am still depending on this implementation with its awesome speed.
Is there any chance to get it over the finish line and merge it? Is there something I can do to help?

The changes I require to make it work for me personally are added in my fork and affect the ability to load other weather data. Just leaving the link here for reference:
https://github.com/jnettels/demandlib/tree/features/add-vdi-from-lpagg

p-snft · 2023-06-28T12:28:58Z

examples/vdi_profile_example.py

+
+from demandlib import vdi
+
+# The following dictionary has been created by "workalendar"


Suggested change

# The following dictionary has been created by "workalendar"

# The following dictionary can also be created by "workalendar"

Maybe this is more clear.

p-snft · 2023-06-28T12:33:22Z

@jnettels: Honestly, I'm not really deep into demandlib. I'm willing to review and give feedback. If you have an improved version, you can file a PR against this branch.

jnettels · 2023-07-03T15:27:09Z

Thanks for your answer. My "improvements" concerning the weather data were already suggested, once with a rude (still sorry about that!) push to this PR, once with the "suggestion" feature. @uvchik already mentioned that he had a different approach for the weather data, so I do not think creating a new PR for that is helpful now.

uvchik added 10 commits April 6, 2022 18:21

Add lpagg code to demanlib

477c92e

Add example for VDI profiles

4c63562

Fix grammar

73480cd

Add extra requirements for examples

6109b41

Use black style

d512612

Revise region module

ea24008

Make it possible to import Region from demandlib.vdi

7b258b8

Add function to find TRY region by coordinates

ab21b5b

Add geometry as extra requirements

33a5964

Add docstrings

4160146

uvchik self-assigned this Apr 12, 2022

uvchik added 4 commits April 12, 2022 10:37

Remove binary file from repository

a3660ae

Do not read unused columns

3785bc9

Fix style issues

b11468e

Fix style issues

4eeed80

uvchik added the enhancement label Apr 12, 2022

uvchik added this to the v0.2.0 milestone Apr 12, 2022

Use new black style

ac2eb6c

uvchik requested review from p-snft, jnettels and joroeder April 12, 2022 14:53

Use temperature limits from config file

d9e0d8d

uvchik added 3 commits April 13, 2022 22:33

Add missing config import

31640f7

Fix time stamp to left index

00e1114

Fix filepath for TRY data

2e205c9

Remove blank line

f5a6e78

uvchik linked an issue Apr 14, 2022 that may be closed by this pull request

Cooperate with lpagg #36

Open

jnettels reviewed Apr 29, 2022

View reviewed changes

p-snft requested changes May 2, 2022

View reviewed changes

Make VDI header clearer

5d75001

Temperature limit is part of the house data

7cccf52

The temperature limit is connected to the efficiency status of the house.

uvchik force-pushed the features/add-vdi-from-lpagg branch from 85d4768 to 7cccf52 Compare June 21, 2022 10:32

p-snft requested changes Jun 30, 2022

View reviewed changes

jnettels reviewed Jul 1, 2022

View reviewed changes

p-snft requested changes Jul 5, 2022

View reviewed changes

p-snft marked this pull request as draft July 5, 2022 07:11

Update src/demandlib/vdi/regions.py

a54ca63

Co-authored-by: Joris Zimmermann <[email protected]>

Krien added 2 commits September 6, 2022 20:24

Fix deprecated warning

652ef05

Make it possible to fetch weather data by knowing the TRY region

b3fd0c4

p-snft reviewed Sep 7, 2022

View reviewed changes

p-snft reviewed Jun 28, 2023

View reviewed changes

	# The following dictionary has been created by "workalendar"
	# The following dictionary is created by "workalendar"

		from .dwd_try import find_try_region # noqa: F401
		from .regions import Region # noqa: F401

		summer=house["summer_temperature_limit"],
		winter=house["winter_temperature_limit"],

-            The value will be passed to the pandas resample method.
+            The value will be passed to the pandas resample method.
+        file_weather : str (optional)
+            Path to a 'test reference year' (TRY) weather file by German DWD
+            (Deutscher Wetterdienst). If None, the file fitting the given
+            try_region will be loaded.

	self.weather = None
	self.weather = None
	self.file_weather = file_weather

		# typtage_combinations = settings["typtage_combinations"]
		# houses_list = settings["houses_list_VDI"]

		# df = pd.concat([df_typ, mytime], axis=1).ffill()
		# df.drop(df.head(1).index, inplace=True)


		from demandlib import vdi

		# The following dictionary has been created by "workalendar"

	# The following dictionary has been created by "workalendar"
	# The following dictionary can also be created by "workalendar"

Add VDI profiles based on lpagg #50

Are you sure you want to change the base?

Add VDI profiles based on lpagg #50

Conversation

uvchik commented Apr 12, 2022 • edited Loading

jnettels commented Apr 13, 2022 • edited Loading

uvchik commented Apr 13, 2022 • edited Loading

pep8speaks commented Apr 13, 2022 • edited Loading

Comment last updated at 2022-09-06 18:25:57 UTC

jnettels commented Apr 27, 2022 • edited Loading

Choose a reason for hiding this comment

p-snft left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jnettels commented Jun 20, 2022

jnettels commented Jun 30, 2022

p-snft left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jnettels left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

p-snft left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

p-snft commented Jul 5, 2022

uvchik commented Jul 12, 2022

jnettels commented Jul 14, 2022

Choose a reason for hiding this comment

jnettels commented Jun 17, 2023

Choose a reason for hiding this comment

p-snft commented Jun 28, 2023

jnettels commented Jul 3, 2023

uvchik commented Apr 12, 2022 •

edited

Loading

jnettels commented Apr 13, 2022 •

edited

Loading

uvchik commented Apr 13, 2022 •

edited

Loading

pep8speaks commented Apr 13, 2022 •

edited

Loading

jnettels commented Apr 27, 2022 •

edited

Loading

p-snft left a comment •

edited

Loading