impactlab / eemeter Goto Github PK

Since PrePostMeter automatically separates inputs and appends "_pre" and "_post" to the key names, separating the pre/post meter will be necessary for submeters which only expect to see one or the other, not both.

Meters are quite flexible - should we force base meters to conform to a particular set of options or sequences? Pros: may make it easier to know what you're getting when you use a new meter, etc. Cons: reduces flexibility, makes it more difficult to update the skeletal structure of a meter.

Some of the tests that use lists of consumptions and consumption histories could be refactored to DRY things up.

For significant speed increases, maybe just keep in memory.

Spot check weather data downloading to make sure the data hasn't drastically changed.

The current tutorial leaves this out, and that makes it difficult for people to try it out on their own data and understand how to use the eemeter in their own workflows.

Where should this fit in? It would be nice to be able to visualize projects and model parameters.

Someone might unwittingly specify to use Celsius in a consumption generator and use a default like Fahrenheit in a weather model. This error would probably be tough to track down.

The PRISM model only pays attention to HDDs

The generated API documentation is a bit tricky to read and navigate.

Some docstrings are out of date.

Output of the get_input function is a recursively structured dictionary which might be bulky and hard to parse, especially when all one wants to know are what keyword args to provide.

Right now the setup.py install_requires list has versions specified, but it might be too strict.

Instead of defining a get_outputs function similar, it is probably better to just include this in docstrings. But we should discuss the pros and cons.

E.g.

 File "/opt/python/run/venv/src/eemeter/eemeter/models/temperature_sensitivity.py", line 103, in compute_usage_estimates
    total_usage = np.sum(cooling + heating) + base_consumption*interval_daily_temps.shape[0]
 AttributeError: 'list' object has no attribute 'shape'

Haversine function finds distance between lat/long pairs.

Consumption data carries energy type and units along with it. Consider restructuring into something like this?

ct = ConsumptionType("electricity","kwh")
Consumption(ct,12.1,datetime(2011,1,1),datetime(2011,2,1))

Since weather sources really need only act on DatetimePeriods, this makes more sense and should allow greater flexibility while minimizing accidental dependencies on the non-DatetimePeriod-related aspects of Consumption instances.

See the specification linked in the docs.

Generators should use "model" classes internally in order to:

• use the same model parameters
• make the internal model explicit
• facilitate customization and modularity
• avoid code repetition

Get misleading KeyError: 'outputs' on call to NREL API made with a bad API Key.

It currently treats all consumption periods as if they were the same length.

E.g. Inherited methods are not copied into documentation for GSODWeatherSource

There was some discussion about whether calling structural meter classes (which also inherit from BaseMeter), should retain "Meter" in the name. For instance, which is better - SquentialMeter or Sequence? Does retaining "Meter" in the name make it easier or harder for newcomers to understand the meters they need to use? Does it help contributors contribute?

DoubleBalancePointModel and PRISMModel both add baseline daily usage monthly rather than daily. This means that consumption periods of differing length will both have the same baseline usage added to them, rather than baseline * n_days.

Right now it uses a global optimization of parameters.

Weather sources which download data should use io instead of StringIO

If positional arguments are provided instead of keyword args, errors are fairly obscure, although they are the standard python argument TypeError, e.g.:

TypeError: evaluate() takes exactly 1 argument (2 given)

Some users have indicated a desire to use python 3; make sure we write python 3 compatible code in the future and convert the current code to be compatible.

Weather data, including observational data, daily summaries, and weather normals often need to be collected for building performance analysis. Because needs across the library will vary widely - with some users performing statewide or nationwide studies and other users performing analyses on only one or two projects - data caching and storage must be efficient enough to work at scale or across multiple machines without being so cumbersome that it scares away users with simpler needs. We need to develop a flexible weather data caching strategy.

e.g. HDDCDDBalancePointModel to TemperatureSensitivityModel(cooling=True,heating=True)

One of the downsides to using a (**kwargs) format for specifying meter parameters is that python's built-in error handling doesn't give very helpful errors. We'll have to make a better format for raising errors in meters for missing parameters, incorrect initialization, etc.

Meter inputs and outputs should be more explicit - and this may be a good way to make that happen.

potash:
Using this TMY3 file: http://rredc.nrel.gov/solar/old_data/nsrdb/1991-2005/data/tmy3/722880TYA.CSV

I get assertion errors in test_meter_content like:

assert arrays_similar(array([  1.51781322e-02,   2.57462824e-02,   7.85082316e+00,\n         6.50000000e+01,   2.00000000e+00]), [0.013456, 0.029507, 8.341199, 65, 3.8])

assert arrays_similar(array([  1.69555227e-02,   4.27854871e-02,   6.00419956e+00,\n         6.50000000e+01,   3.30000000e+00]), [0.016883, 0.042749, 6.013131, 65, 3.3])

Note that they are about equal to 2 significant digits. So I'm assuming my TMY3 file is slightly different from the one used to write the tests? Can you confirm that?

If so, one solution is to bundle or link to the exact TMY3 file needed for those numbers with ee-meter. Thoughts?

Right now we have dependencies on fairly static web APIs that go from zipcode to lat long to weather station. This data doesn't take up more than a couple of MB, so it may be worth just storing compressed json files with this info as an internal resources, making the eemeter completely standalone.

Naming should then be more accurate and descriptive.

We no longer need to use these functions, as all zipcode/weather station/lat long wrangling is internal now.

One of the main readability issues with meter specifications is that they don't require explicit input and output mappings. If they were required, they would be more verbose (and sometimes trivial) but more easily readable. Thoughts on this?

This will allow us to compare parameters estimates in a meaningful way.

Meters generally expect particular types of inputs, so it might be good to generate flags automatically for null inputs to the meter (such as meter.run(consumption_history=None)).

The alternative (current implementation) is to throw an error. For now, a meter can't be run unless all inputs are supplied. Some inputs will be missing, though, and the meter should still be able to run. One way or another these errors should be made more readable.

Some GSOD stations do not have data for all days (e.g. 724936, 2010-01-01). When this occurs, there is a KeyError which is uncaught.

NREL's Uniform Methods
http://energy.gov/sites/prod/files/2013/11/f5/53827-8.pdf

We should make sure the OEEM is inline with this. sorry i don't know how to add this to docs page

All outputs of meters must be dealt with - it might be nice to have some syntax for removing them from the flow, or of adding new inputs on the fly. Removing could be as simple as

output_mapping = {"output1": None, ...}

Can view the latter are special cases of the former, or the former is the sum of the latter. Either way, it's easier to maintain the code if the HDD and CDD calculations are only in one place.

It just needs to give an example of, say, connecting to a database.

Some homes with net metering have negative consumption base loads. This is not handled by the parameter ranges in the prism meter.

The __init__ function of the ModelBase implies that x0,bounds will be necessary for most model instances; however, this is not true for generators, for which these two values are meaningless. Either refactor or make them default to None.

We have a HDDCDDBalancePointModel, and a HDDBalancePointModel, so why not.