impactlab / eemeter

‼️ MOVED TO https://github.com/openeemeter/eemeter - Core computation engine for the Open Energy Efficiency Meter
https://eemeter.readthedocs.io/
MIT License
25 stars 13 forks source link

Metric model inputs #78

Closed potash closed 9 years ago

potash commented 9 years ago

Metrics are currently incorporated into meters in two distinct ways:

We'll probably want to use the former for some set of core metrics and the latter for everything else.

However, since the TemperatureSensitivityParameterOptimization does not return estimated usages, daily temperatures etc., these must be refetched/computed in the latter approach leading to duplication and inefficiency. As we add more metrics this will be a problem.

Is there any reason why TemperatureSensitivityParameterOptimization shouldn't just return everything (consumptions, average daily usages, observed daily temps, weights, estimated_daily_usages)? Alternatively we could have a intermediate ModelEvaluationMeter (or something) that calculates that stuff and feeds it to all the metric meters.

philngo commented 9 years ago

Thanks for bringing this up and sorry for taking so long to respond. You make some excellent points. The current implementation of CVRMSE is unnecessarily inefficient because it recomputes and gathers observed_daily_temps, estimated_daily_usages, etc. If the many other metrics we are soon to incorporate into meters all worked that way, we will quickly start to see significant performance decreases.

Restating what you wrote above: for any single metric, we have a choice between

  1. hard-coding the metric into the meter
  2. passing the metric into meter (using some sort of interface we haven't yet defined), potentially with a list of other metrics, or
  3. exporting the intermediate values from the optimization meter and using them later when we want to compute metrics.

I think we will probably want to save option 1 for metrics which are particularly associated with the optimization method itself. That's probably the bare minimum set of "core metrics", and I'm not sure we should expand it.

Option 2 is intriguing but probably quite challenging; it requires the definition of a new interface. A nice interface for metrics might make it easier for contributors to define new metrics and incorporate them into new or existing meters. However, I'm worried first that it will be impossible to fit all metrics into a succinct interface and second that the metric class will end up being too specific to the temperature sensitivity parameter optimization meter.

I like Option 3 the best for most metrics. One of the principles of the open ee meter is that results should be transparent and repeatable. Since the intermediate values you mentioned before are actually quite central to the meter; they should probably be exported anyway. (In answer to your question, there is a pretty good reason we should just return everything. I think we value transparency and repeatability more than we object to clutter.) Later on in the meter execution sequence, we can just look for the values we want and make them inputs to the metrics. Another reason I like this method is that it doesn't close us off to the option of packaging metrics with meters if we want to or need to in later versions.

philngo commented 9 years ago

To use option 3, we would simply make the interface accept y_hat and y, then simply map in the correct values which were exported from the TSPOmeter (average_daily_usages ==> y, estimated_daily_usages ==> y_hat)

I'm going to go ahead and implement that, because that seems pretty clean.

philngo commented 9 years ago

73819b4a98dae640a91a677d3e51f9022f834c01 implements this change (and a bunch of other minor changes) for CVRMSE. Next step should be to repeat this for the standard error, and other various metrics, (flatness, r^2, etc).

philngo commented 9 years ago

6d8d8e7e05ae49ace29630b37083c6af74286c63 uses new method to calculate RMSE (replacing the daily_standard_error metric which was internally calculated in the TSPOmeter.