LBNL-ETA/MSWH

Exceptions in example notebooks

brynpickering opened this issue · 6 comments

I just tried running the following notebooks from the scripts folder: 'MSWH System Tool.ipynb', 'MSWH System Tool - Playground.ipynb', and 'MSWH System Tool - for CEC Report.ipynb' and couldn't get them to run in their entirety, namely:

  1. they depend on seaborn, which isn't in the mswh requirements
  2. they raise an exception in the Individual retrofit solar thermal system section (~cell 55): ERROR:mswh.system.models:Check project output of the backup - timestep sum preformed in this method and the total are not equal.
  3. There's also another exception in 'MSWH System Tool - Playground.ipynb', cell 38: NameError: name 'cold_cz' is not defined

Note: this issue comes to you as part of my wider JOSS review

Thanks @brynpickering! I will add seaborn to the setup.py

Was the MSWH System Tool.ipynb running fine? The playground does not need to be functional at all times, so I may move it to a different branch.

They all fail at the same point, including MSWH System Tool.ipynb. I would definitely suggest cleaning out the project-specific outputs into branches, since they cause a bit of confusion at the moment! It will also allow you to pin the MSWH version to be compatible with that project's notebook.

Sure! Would you mind me asking what do you exactly mean with pin. Would it be sufficient to tag the repo and provide information about the tag in the notebooks?

@brynpickering please see the PR I have created. I addressed all but one of the issues, that is ERROR:mswh.system.models:Check project output of the backup - timestep sum preformed in this method and the total are not equal.. This is not showing on my end. Let's figure out what is going on. Thanks!

For completeness, here's the full error trace.

CLICK ME

INFO:mswh.system.models:Assigned weather data timeseries.
INFO:mswh.system.components:Assigned weather data timeseries.
INFO:mswh.system.components:Solar storage tank is set.
INFO:mswh.system.components:Gas tank WH (WHAM) is set up.
ERROR:mswh.system.models:Check project output of the backup - timestep sum preformed in this method and the total are not equal.
---------------------------------------------------------------------------
Exception                                 Traceback (most recent call last)
<ipython-input-53-dd1581f8f780> in <module>()
      7                           loads = loads_indiv)
      8 
----> 9 cons_total_indiv_retrofit_cold, proj_total_indiv_retrofit_cold,[proj_total_dict_indiv_retrofit_cold, sol_fra_indiv_retrofit_cold, pump_el_use_indiv_retrofit_cold, pump_op_hour_indiv_retrofit_cold, ts_res_indiv_retrofit_cold, backup_ts_cons_indiv_retrofit_cold, rel_err_indiv_retrofit_cold] =     sol_wh_indiv_retrofit_cold.solar_thermal(backup = 'retrofit')

/path/to/MSWH/mswh/system/models.py in solar_thermal(self, backup)
    463                   'not equal.'
    464             log.error(msg)
--> 465             raise Exception
    466 
    467         # household results (annual quantities split according

Exception: 

Anyway, debugging it, it seems to be a floating point error. These are the two values being compared:

ipdb>  proj_total[self.r['gas_use']]
1824376.782528877
ipdb>  backup_proj_total[self.r['gas_use']]
1824376.7825287068

I would suggest using an approximate comparator for floats, like math.isclose() to help avoid these issues.

On pinning, what I mean is that you know your code works with a particular notebook for a particular project. As you develop the code further, you want that notebook to continue working without needing to constantly update it. One way of doing this is to branch off and then leave that branch alone. If you just tag the master branch and leave that notebook in the master branch then you should really maintain the notebook alongside the package code.

The question of whether these notebooks should even be here is also relevant (see your question in #9 on seaborn as a dependency), but to be dealt with there / in another issue I think.

Thanks.