Add modest-py fmu-based parameter estimation

[dhblum]

This issue is to architect the use of fmu-based parameter estimation using https://github.com/sdu-cfei/modest-py.

[krzysztofarendt]

Thanks, I created a branch for this issue: https://github.com/lbl-srg/MPCPy/tree/issue47_add_modestpy

[dhblum]

@krzysztofarendt @LisaRivalin I've merged the latest master to this branch.

[dhblum]

@krzysztofarendt I recommend you merge the latest master with refactored unittests. Note the function check_df_timeseries and check_df_general have been merged into one check_df with a boolean argument of timeseries.

[dhblum]

Do you have the Modelica Buildings Library path on your MODELICAPATH environmental variable?

On Thursday, November 16, 2017, Krzysztof Arendt notifications@github.com wrote:

@dhblum https://github.com/dhblum, I haven't merge yet, because 6 tests are failing for me. The tests that are failing are using this model: mpcpy/unittests/resources/model/LBNL71T_MPC.mo.

The unit tests fail due to compilation errors, similar to this one (there are many of them):

Error: in file '/home/krza/link/code/mpcpy/unittests/resources/model/LBNL71T_MPC.mo': Semantic error at line 882, column 23: Cannot find class or component declaration for H

Failing tests:

test_estimate_and_validate (unittests.test_models.EstimateFromJModelicaEmulationFMU) test_simulate_initial_parameters (unittests.test_models.EstimateFromJModelicaEmulationFMU) test_estimate_and_validate (unittests.test_models.EstimateFromJModelicaRealCSV) test_simulate_initial_parameters (unittests.test_models.EstimateFromJModelicaRealCSV) test_energycostmin (unittests.test_optimization.OptimizeAdvancedFromJModelica) test_energymin (unittests.test_optimization.OptimizeAdvancedFromJModelica)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/lbl-srg/MPCPy/issues/47#issuecomment-344883544, or mute the thread https://github.com/notifications/unsubscribe-auth/ASlBpx0UCosiUCno0jjkyXoZeEvwGiYDks5s3BCEgaJpZM4ODvAy .

[krzysztofarendt]

Dave, I added one unit test for ModestPy: EstimateFromModestPyRealCSV. Please take a look and let me know if this is the right approach. I also exposed tolerance as an argument to check_df, because I thought it might be useful if you want limit the computational time of the tests. The new unit test takes 4-5 minutes to complete, but it could be limited by increasing the tolerance.

I also have a comment to other unit tests in test_models.py. In EstimateFromJModelicaRealCSV.test_estimate_and_validate() and EstimateFromJModelicaEmulationFMU.test_estimate_and_validate() you check whether simulated data are equal to the reference data from simulate_estimated_parameters.csv:

# Check references
df_test = self.model.display_measurements('Simulated');
self.check_df(df_test, 'simulate_estimated_parameters.csv');

However you do not simulate the model in this unit test. Instead, you simulate it in another unit test test_simulate_initial_parameters(). If that's correct, then you are breaking the encapsulation rule of the unit tests, which in this specific case might be reasonable, but in general makes it more difficult to conclude what fails in the code. Anyway, how do you make sure that test_simulate_initial_parameters() always runs first? If test_estimate_and_validate() runs first, then you get an error that there is no simulated data in the measurements.

For this reason I combined both unit tests into a one in EstimateFromModestPyRealCSV.

[dhblum]

I don't see how changing the tolerance will change the computational time of the test, unless the testing of values of a timeseries terminates early because an error is found greater than the tolerance. I'd like to keep the tolerance value of 1e-3 as it is for now, which is based on the tolerance used in regression tests of the buildings library using https://github.com/lbl-srg/BuildingsPy.

Thank you for pointing out the other unit tests in test_models. There are a couple things going on here at once. First, the tests are not intended to be run in a particular order. It is a typo that the model is not simulated directly after estimation. I will make this fix. The 'Simulated' measurements that are being checked are actually the results of the initial simulation used for the initial guess of the optimization in the estimation. I'm considering this a bug and is part of what will be addressed in https://github.com/lbl-srg/MPCPy/issues/93.

I'd still like to test that the initial guess simulation results are consistent. I suggest simulating the model before estimation, checking the results, estimating, then simulating again, and then checking the new results. Essentially, as you suggested, combining the tests into one, but adding a simulation of the model before estimation:

# Simulate initial parameters
self.model.simulate(self.start_time_emulation, self.final_time_emulation);
# Check references
df_test = self.model.display_measurements('Simulated');
self.check_df(df_test, 'simulate_initial_parameters.csv');
# Estimate parameters
self.model.estimate(self.start_time_estimation, self.final_time_estimation, self.measurement_variable_list);
# Simulate with estimated parameters
self.model.simulate(self.start_time_emulation, self.final_time_emulation);
# Check references
df_test = self.model.display_measurements('Simulated');
self.check_df(df_test, 'simulate_estimated_parameters.csv');

I've also noticed that start/final_time_emulation is not needed, only _estimation and _validation. I've opened a new issue to perform cleaning not related to ModestPy integration. See https://github.com/lbl-srg/MPCPy/issues/94.

[dhblum]

https://github.com/lbl-srg/MPCPy/issues/94 closed via https://github.com/lbl-srg/MPCPy/pull/103.

[dhblum]

The closing and reopening of this issue was accidental. It is not closed.

[dhblum]

@krzysztofarendt please check if you agree with the latest commits to this branch. Please also check that the unit test passes on your machine with the reference results I've committed.

I think the implementation is looking good and we are ready to merge to master very soon.

[krzysztofarendt]

Thanks @dhblum for your help on it. I run unit tests and everything works. I agree with the changes with one comment:

models.py, lines 1015-1017

# Learning period
ideal = ideal[start:end] 
inp = inp[start:end]

df[start:end] excludes the end row. If you prefer to include it, use df.loc[start:end].

[dhblum]

Great, thanks.

Good catch, I will implement df.loc[start:end]. This is how it is implemented elsewhere in MPCPy.

Two questions regarding the options of ModestPy: 1) sqp_opts is listed as an optional parameter in the docstring, and included in the #Custom settings from kwargs section, but is not set in the instantiation of modestpy.Estimation. 2) ic_param is not listed as an optional parameter in the docstring, but is included in the #Custom settings from kwargs section and is set in the instantiation of modestpy.Estimation.

For 1) I recommend adding sqp_opts to the instantatiation.
For 2) we could keep ic_param as an option and add to doc string. My only concern is confusion if a user specifies an initial condition parameter in the parameter_data dictionary and also defines ic_param arguments. What do you think? I suppose this can also happen in ModestPy if a user specifies an initial condition parameter in the known dictionary.

[krzysztofarendt]

I added sqp_opts and changed to .loc[] slicing: https://github.com/lbl-srg/MPCPy/commit/67d9420367770f9c6d8e1975475f10630f4bfa0c.

As for ic_param, I think it could be handled inside MPCPy some day ;-) It helps to automate the estimation process. Without it, if users wish to estimate on different learning periods they have to manually update parameters outside MPCPy. In real applications the models will have to be re-calibrated periodically.

In ModestPy it's implemented as follows:

1. ic_param is a dictionary containing mapping between model parameters (used to set initial condition for chosen states) and measured outputs.

2. Before each simulation, overwrite selected parameters using the first value from the corresponding measured output.

But as for now, I agree that it would be confusing to add ic_param just in the ModestPy method.

[dhblum]

Agree that it could be implemented wholistically in MPCPy. I do not want to implement that in this issue, however. For now, let's not implement the ic_param argument for ModestPy.

Before merging we need to add some documentation, for use and installation, such as adding modestpy to the list of python packages.

[krzysztofarendt]

I can take care of it. Please check if this list is complete:

• [x] Add ModestPy to Section 1.2: Third-Party Software
• [x] Add ModestPy to Section 2.1: Installation Instructions For Linux
• [x] Add ModestPy to Section 2.2.4: Estimate Parameters
• [x] Add ModestPy to Section 6.1.2: Estimate Methods

I'm not sure what's the best way to modify 2.2.4. Maybe a subsection 2.2.4.1: Alternative Estimation Methods? That may be a good choice if you plan to add more methods later. Although replacing JModelica to ModestPy is as easy as changing one argument in the model instantiation, each alternative method will likely have a list of optional parameters to document.

I will also have to

• [x] update ModestPy on PyPi

[dhblum]

I added 6.1.2 to the check list. You can do this by adding the ModestPy class in the docstring of mpcpy/models.py in the section:

Estimate Methods
================

.. autoclass:: mpcpy.models.JModelica

.. autoclass:: mpcpy.models.UKF

For 2.2.4 you could either reinstantiate the model with the modestpy estimation method or demonstrate the use of the set_estimate_method() to change the estimation method of the model to modestpy and then re-run the estimate. It is ok to add as a new section 2.2.4.1 or not. In the end, users can be referred to Section 6.1.2 for all of the alternative methods and their documentation, which can agreeably be improved.

[krzysztofarendt]

I updated ModestPy on PyPi. Current version: 0.0.7.

[krzysztofarendt]

ModestPy added to Section 6.1.2: https://github.com/lbl-srg/MPCPy/commit/66dc1991e2aeeb2efabad309cc7ac30294d38c79 and https://github.com/lbl-srg/MPCPy/commit/b19fde9da6de4d22a7009f732ecdd8d30673f228

[krzysztofarendt]

ModestPy added to Section 1.2: https://github.com/lbl-srg/MPCPy/commit/bee632eaf8e3052e80986f817c1a7611f0c2a024

[krzysztofarendt]

ModestPy added to Section 2.1: https://github.com/lbl-srg/MPCPy/commit/53cfa915c2ed36ac8d0dc86e2430fcf0b06d1558

I changed the version specifier in tzwhere and modestpy to == to match the syntax used in pip install, i.e. pip install modestpy==0.0.7.

[krzysztofarendt]

ModestPy added to Section 2.2.4: https://github.com/lbl-srg/MPCPy/commit/82c11c8defe61290224eb64235fb0821f1c0c0de

@dhblum, I enabled a sphinx extension for automatic links to other sections ('sphinx.ext.autosectionlabel' added to extensions in conf.py), because I wanted to refer to the section "Models" in the introductory tutorial:

The allowed optional arguments for each method are discussed in Models.

The extension worked for me in this example, but after enabling it I got some warnings in other places in the documentation:

WARNING: /home/krza/code/mpcpy/doc/userGuide/source/exodata.rst:100: (WARNING/2) autodoc: failed to import class u'InternalFromDF' from module u'mpcpy.exodata'; the following exception was raised:
Traceback (most recent call last):
  File "/home/krza/github/virtualenv/mpcpy/local/lib/python2.7/site-packages/sphinx/ext/autodoc.py", line 664, in import_object
    obj = self.get_attr(obj, part)
  File "/home/krza/github/virtualenv/mpcpy/local/lib/python2.7/site-packages/sphinx/ext/autodoc.py", line 554, in get_attr
    return safe_getattr(obj, name, *defargs)
  File "/home/krza/github/virtualenv/mpcpy/local/lib/python2.7/site-packages/sphinx/util/inspect.py", line 185, in safe_getattr
    raise AttributeError(name)
AttributeError: InternalFromDF
/home/krza/code/mpcpy/mpcpy/exodata.py:docstring of mpcpy.exodata:58: WARNING: duplicate label classes, other instance in /home/krza/code/mpcpy/doc/userGuide/source/models.rst
/home/krza/code/mpcpy/mpcpy/exodata.py:docstring of mpcpy.exodata:92: WARNING: duplicate label classes, other instance in /home/krza/code/mpcpy/doc/userGuide/source/exodata.rst
/home/krza/code/mpcpy/mpcpy/exodata.py:docstring of mpcpy.exodata:118: WARNING: duplicate label classes, other instance in /home/krza/code/mpcpy/doc/userGuide/source/exodata.rst
/home/krza/code/mpcpy/mpcpy/exodata.py:docstring of mpcpy.exodata:140: WARNING: duplicate label classes, other instance in /home/krza/code/mpcpy/doc/userGuide/source/exodata.rst
/home/krza/code/mpcpy/mpcpy/exodata.py:docstring of mpcpy.exodata:166: WARNING: duplicate label classes, other instance in /home/krza/code/mpcpy/doc/userGuide/source/exodata.rst
/home/krza/code/mpcpy/mpcpy/exodata.py:docstring of mpcpy.exodata:196: WARNING: duplicate label classes, other instance in /home/krza/code/mpcpy/doc/userGuide/source/exodata.rst
/home/krza/code/mpcpy/mpcpy/exodata.py:docstring of mpcpy.exodata:231: WARNING: duplicate label classes, other instance in /home/krza/code/mpcpy/doc/userGuide/source/exodata.rst
looking for now-outdated files... none found
pickling environment... done
checking consistency... /home/krza/code/mpcpy/doc/userGuide/source/units.rst: WARNING: document isn't included in any toctree
/home/krza/code/mpcpy/doc/userGuide/source/utility.rst: WARNING: document isn't included in any toctree

[dhblum]

Thanks for this. I added some comments to https://github.com/lbl-srg/MPCPy/commit/82c11c8defe61290224eb64235fb0821f1c0c0de and will look into the warnings generated.