SEC-i / ecoControl

ecoControl optimizes the use of heterogeneous energy systems in residential buildings.
http://sec-i.org/ecocontrol/
MIT License
13 stars 4 forks source link

Use array.array in take_and_cache function #7

Closed fniephaus closed 10 years ago

fniephaus commented 10 years ago

Lists are a bottleneck when it comes to collecting a huge amount of data very often. It does make sense to use array.array instead and calculate all times on demand.

I assume that this could speed up things a lot...

This is how it looks now:

self.forecast_data[index].append(
     [datetime.fromtimestamp(self.env.now).isoformat(), round(float(value), 2)])
MaxReimann commented 10 years ago

Better always do some profiling first ;)

Line Profiling gives me:

Line       Hits         Time  Per Hit   % Time  Line Contents
==============================================================
   131                                               def step(self):
   132                                           
   133      1344        37754     28.1      0.6          execute_user_function(self.env,self.env.forecast,self.devices,self.user_function)
   134                                           
   135      1344         7893      5.9      0.1          if self.use_optimization and self.next_optimization <= 0.0:
   136                                                       auto_optimize(self)
   137                                                       self.next_optimization = 3600.0
   138                                           
   139                                                   # call step function for all devices
   140      9408        44685      4.7      0.7          for device in self.devices:
   141      8064      1610482    199.7     23.9              device.step()
   142                                           
   143      1344      5024289   3738.3     74.5          self.take_and_cache()
   144                                           
   145      1344         9807      7.3      0.1          self.env.now += self.env.step_size
   146      1344         9045      6.7      0.1          self.next_optimization -= self.env.step_size

File: d:\WorkSpace\ecoControl\server\forecasting\measurementstorage.py
Function: take_and_cache at line 52
Total time: 0.975381 s

Line       Hits         Time  Per Hit   % Time  Line Contents
==============================================================
    52                                               def take_and_cache(self):
    53     17472        91947      5.3      2.3          for index, sensor in enumerate(self.sensors):
    54    112896       482269      4.3     12.1              for device in self.devices:
    55     96768      2462399     25.4     61.5                  if device.id == sensor.device.id and sensor.in_diagram:
    56      9408        69006      7.3      1.7                      value = getattr(device, sensor.key, None)
    57      9408        36569      3.9      0.9                      if value is not None:
    58                                                                   # in case value is a function, call that function
    59      9408        46009      4.9      1.1                          if hasattr(value, '__call__'):
    60      9408       727513     77.3     18.2                              value = value()
    61      9408        85130      9.0      2.1                          self.forecast_data[index].append(float(value))

So its actually line 55 and line 60 costing very much performance.. I assume that line 55 has to do something with database lookups, so we should rewrite it or rearrange the loops

fniephaus commented 10 years ago

This is good work!

I think select_related is what we want to do: https://docs.djangoproject.com/en/dev/ref/models/querysets/#select-related I'll try and fix it on the line_profiling branch, can you rerun the profiling again then?

MaxReimann commented 10 years ago

Sure, I'll do that tomorrow

fniephaus commented 10 years ago

I think I fixed it. Was able to run it myself, but I'll keep this up to you. Not sure what we can do about line 60 though...

MaxReimann commented 10 years ago

I further improved the loop performance

   139                                                   # call step function for all devices
   140      9408        42706      4.5      1.0          for device in self.devices:
   141      8064      3282047    407.0     73.7              device.step()
   142                                           
   143      1344      1069199    795.5     24.0          self.store_values()
   144                                           
   145      1344         9255      6.9      0.2          self.env.now += self.env.step_size
   146      1344         8064      6.0      0.2          self.next_optimization -= self.env.step_size

File: d:\WorkSpace\ecoControl\server\forecasting\measurementstorage.py
Function: take_and_cache at line 61
Total time: 0.213891 s

Line #      Hits         Time  Per Hit   % Time  Line Contents
==============================================================
    61                                               def take_and_cache(self):
    62     10752        49661      4.6      5.7          for index, (sensor, device) in enumerate(self.device_map):
    63      9408        55333      5.9      6.3              value = getattr(device, sensor.key, None)
    64      9408        33007      3.5      3.8              if value is not None:
    65                                                           # in case value is a function, call that function
    66      9408        40969      4.4      4.7                  if hasattr(value, '__call__'):
    67      9408       622859     66.2     71.0                      value = value()
    68      9408        75513      8.0      8.6                  self.forecast_data[index].append(float(value))

I dont think there's much we can do about value() as it sometimes needs to call expensive functions (like retrieving the weather).

Maybe the devices step() function can be improved by using numpy.. but I wouldnt focus on that for now. I think this issue can be closed.

fniephaus commented 10 years ago

Let's not overdo it :+1: