Issue146 pyfmi3 cs

[dhblum]

This for #146. In particular, it:

• Updates the BOPTEST docker container to use Python 3, the standalone pyfmi package, and co-simulation FMUs, which better addresses #213 and #274. There are small, though insignificant, changes to KPIs for some test case models. Simulation time of the test cases improves for some of the smaller models, though increases for the larger ones. The increase is not unrealistic however, and I believe the improvements of these changes with respect to maintainability moving forward are worth it.
• Remove plotting functionality from kpi calculator, as it is has not been used and is misleading with lots of extra code.
• Remove root makefile, as it has been deprecated for some time and would require maintenance for this PR.

To still be completed:

• [x] Update unit test reference results
• [x] Need to split unit tests more since taking too long for travis
• [x] Update test case fmus
• [x] Update release notes
• [x] Resolve commented code in kpi calculator
• [x] Complete reviews

@JavierArroyoBastida Can you take a look and provide a review?

[dhblum]

Thanks @JavierArroyoBastida. I will check on a few things as noted in the comments. I can also provide plots from the reference comparison in terms of trajectories and KPIs. Correct, I don't see a reason why results should be trusted more with ME. In fact, as I noted in the original comment, I think CS is a better solution for https://github.com/ibpsa/project1-boptest/issues/274.

[dhblum]

@JavierArroyoBastida I'm attaching a zip file containing many trajectory plots and KPI comparisons from compare_references.py for all test cases and associated scenarios. The bestest_hydronic_heat_pump also contains the results from the event update unit test. Please review. I've looked at a bunch a noted negligible differences in KPIs and result trajectories, except for the computational time ratio KPI, which is a proxy measure for simulation time, where some test cases run faster, some slower.

boptest_pyfmi3_benchmakring_plots.zip

[javiarrobas]

Thanks a lot for sharing the KPI result and trajectory comparisons @dhblum! I'm very happy to see such small numerical differences, which are also negligible in my opinion. Note however that I don't think the computational time ratio can be considered a proxy measure for simulation time since it only talks about the computational time of the external controller. The time of the controller is divided by the test case step period, which is not to be confused with the time that it takes the emulator to simulate that step. I think it's relevant to get a notion of the change in simulation time when going from model exchange to co-simulation, but I guess that needs to be explicitly measured.

[dhblum]

@JavierArroyoBastida My thinking was the step time doesn't change from the unit tests in master to the unit tests here. So I agree the absolute value isn't a true measure of computational time, but I think if the comp time ratio is double, that's an indication the absolute simulation time is double. I can post more explicit benchmarks though.

[javiarrobas]

You're right that the step time doesn't change from the unit tests in master to the unit tests here. Hence, if the comp time ratio is double, that indicates that the time that it took to compute the control action to the external controller is double for whatever reason. However, the simulation time of the emulator could still be 10 times longer, or 10 times faster, which I don't think that would be reflected in the comp time ratio at all. I therefore think that a more explicit measure is needed.

[dhblum]

Ok yes I see what you're saying, sorry.

[dhblum]

Ok I completed some simulation time benchmarking. I ran tests to simulate each test case for the scenario "peak heat day" and "dynamic" electricity price using the current implementation and the proposed changes in this PR. Tests were run on a virtual machine with Ubuntu 18, 8 GB Memory, 2 CPU cores (2.6 GHz Intel Core i7) using the attached script. I timed the simulation loop using a two-week step size (so just run the scenario all at once) and once using a 15 min control step in a loop until completion of the scenario (more like what you'd use if testing an external controller). I also timed getting results and reporting the KPIs. I report below the simulation loop times. Attached, find the full results that include the results and KPIs, for which the PR implementation is generally faster. But this isn't the case for simulation for all test cases, though is for some, see below. Note, there is no external controller here, just running baselines. All data in seconds.

profile_me_cs.txt

time_benchmark.zip

[javiarrobas]

Thanks a lot for the analysis @dhblum. Nice to see that some models run faster, and for those that don't, that the difference is diminished when using shorter time steps which is probably the most common use case. As an observation, the models that show slower simulation times are those with multiple zones, and it clearly gets worst when increasing the number of modeled zones: two for the twozone_apartment_hydronic, five for the multizone_office_simple_air, and eight for the multizone_residential_hydronic. Despite the increased simulation time for these models, I'm very much in favor of moving forward with co-simulation because of the reasons you state in this comment and because it draws a more clear line between test case and BOPTEST developments. Having co-simulation FMUs gives test case developers the flexibility to play with the solver if that was needed to improve simulation speed.

[dhblum]

I agree @JavierArroyoBastida that this change is better in the long run. It will also better position BOPTEST to take advantage of computational advances in the tools used to compile and simulate the FMUs. Let me know if you have any other comments and we can also discuss in the upcoming IBPSA BOPTEST meeting. I can also work to finalize the PR.

@kbenne This PR is nearing completion and I think it might be good to test a deployment of this development branch with Service. Is that possible?

[dhblum]

@kbenne Can you test in a Service deployment?

[dhblum]

@kbenne I'm sorry this did not go as smoothly as planned. I updated unit test results after applying the fix for #577, but then realized I needed to merge the latest master, and after that, update unit test results again, since I think the fix from #533 comes into play again. Long story short, you might try updating boptest-service to this PR to see if test pass on your side, but I'd also like to wait until the last travis test results finish from https://github.com/ibpsa/project1-boptest/pull/536/commits/e0058299a06e1e9059b6e6eb62d8e1fd8bb667fe to see if there are further issues to be aware of.

[dhblum]

@kbenne Ok my latest commits addressed the unit test errors reported at https://github.com/ibpsa/project1-boptest/pull/536/commits/e0058299a06e1e9059b6e6eb62d8e1fd8bb667fe. Need to wait for latest tests to finish, but I expect them to pass. Will update if they don't. I suggest continuing with boptest-service sync on this, and let me know how it goes.

[dhblum]

As tests are passing and has been tested with boptest-service, this is ready to merge.