baseline step size

[javiarrobas]

Current master returns slightly different baseline KPI results when using different step sizes for the same scenario and simulation periods. Here the script to reproduce the issue:

test_kpis_baseline.txt

Running this script in master and in https://github.com/ibpsa/project1-boptest/tree/issue270_scenarioTime_2 returns in both cases the exact same results:

tdis_tot
8.29017550098
8.35305147656
cost_tot
167.731006157
167.730361748

Where it's shown a small discrepancy when running the baseline using a step size of 12 hours vs. a step size of 24 hours.
In the beginning I thought it could be a bug from the KPI calculation module but after reviewing it I figured out that this is because the communication points returned from pyfmi depend on the simulation length, which in turn affects the integration to calculate KPIs. This issue can be solved directly by merging https://github.com/ibpsa/project1-boptest/tree/issue240_memory to master, which fixes the number of communication points. After so the script returns:

tdis_tot
8.28941468357
8.28941468357
cost_tot
167.741309909
167.741309909

which proves that baseline KPIs are independent of step size.
My suggestion is to prioritize merging not only https://github.com/ibpsa/project1-boptest/tree/issue270_scenarioTime_2 but also https://github.com/ibpsa/project1-boptest/tree/issue240_memory.

EDIT: I forgot to mention that I used the bestest_hydronic_heat_pump case for all tests.

[javiarrobas]

Below the branch that I used to merge https://github.com/ibpsa/project1-boptest/tree/issue240_memory and that leads to same baseline results independently of the step size:

https://github.com/JavierArroyoBastida/project1-boptest/tree/issue302_baselineStepSize

[dhblum]

Thanks for reporting this. Indeed I reproduced your results, error and fix, on my machine. I merged latest master to https://github.com/ibpsa/project1-boptest/tree/issue240_memory and pushed just to have tests run.

But https://github.com/ibpsa/project1-boptest/tree/issue240_memory makes it so that all trajectories are stored, and reported, at 30s intervals, for instance when using /results. This could be more akin to how sensors typically out at regular intervals. One issue is if a user wants to test at a smaller step size, maybe for a feedback controller.

[javiarrobas]

Tests didn't pass but that's because of small differences and lack of stored event points, which was expected.

I think we need to make a decision here. We can allow the user to set the interval at which trajectories are reported and stored, so that there is no restrictions in that regard. In that case we'd get the small differences in results for different intervals. The other option is to fix the step size, which I'm more in favour of since results' consistency should be a priority and also since sensors may typically output at regular intervals as you well pointed out. Maybe a trade-off is to use 10s fixed interval?

[haraldwalnum]

@dhblum and @JavierArroyoBastida, I cannot resist coming back to the option of integrating the measurements inside the emulator. At least for the energy, emissions and cost KPIs this would solve this issue. For the comfort KPIs it is, as mentioned by Javier in #240, a bit more complicated. Javier proposed one solution. An alternative would be to differentiate the integrated measurement to get the average within the timestep and the use this in the same way as the KPIs are calculated now.

In my opinion, such solution would allow for setting a fixed timestep (for example 30s), that would result in both consistency and reduced memory consumption.

If users need shorter control steps for there controller I guess the current setup will automatically adjust the output interval to the minimum of the predefined output interval and the set control step? This should then not lead to inconsistency in the energy related KPIs, but could lead to small inconsistencies in the comfort KPIs. However I think that when comparing controllers the differences will be so small that a lot of other parameters are considered more important (such as installation complexity, need for sensors etc).

[dhblum]

A couple of comments on options:

1. Setting ncp such that output interval is fixed. I tested setting ncp such that the output interval is 30s but the simulation step is smaller, like 10s. In this case, the output interval looked to be the solver step size. It looks like it is only when the simulation step is larger than the specified output interval that the specified output interval is then used. As @JavierArroyoBastida demonstrated, setting the ncp such that the output interval is 30s removes the integration dependency on step size. The problem would be re-introduced as I've just described if a user sets a control step smaller than 30s. But, as @haraldwalnum points out, the error at this small a time interval would likely be very small.

2. Performing integration inside the model without passing boundary condition data into the model. This could be done for power variables by adding an integrator to the read blocks, passing the output of this block to the wrapped FMU, and using this output in kpi calculation, replacing the power variable in the kpis.json with the integrated variable. (The integral variable may or may not be exposed to the user, depending on whether the emulator developer thinks power meters or energy meters are more appropriate. I think this is a separate discussion.).

• The energy KPI can then be read directly from this integral variable at the current time.
• The cost and emissions KPIs can be calculated as proposed here Memory usage #240 (comment) by using C(t) = p(t-1)*[E(t)-E(t-1)] and then integrating C(t), where p is a price/emission factor and E is the energy.
• The discomfort KPIs calculation proposed here Memory usage #240 (comment) is not correct since it does not take into account the upper and lower temperature limits. But, I think the adjustment is to compute for instance the discomfort from upper temperature limit (_u) as D_u(t) = p(t-1)*{[Y(t)-Y_u(t)] - [Y(t-1)-Y_u(t-1)]} where p = 1 if T(t) > T_u(t), 0 otherwise, Y(t) = integral[T(t)] and Y_u(t) = integral[T_u(t)]. The incorporation of Y_u(t) here should hold true since integral(T-T_u)dt = integral(T)dt - integral(T_u)dt. My math may need to be checked. And the integrated temperature signal would come from the added integrator output in read blocks similar to power.

[javiarrobas]

At this point I'm more in favor of option 1. I understand the point of option 2 and I think that it's a very valid proposal. However, it requires fundamental changes that may generate other issues along the way. One I'm concerned about is the addition of complexity to the emulators, but the worst of all are all those challenges that we can't foresee from here. Integration outside the emulators works fine and has proven to be robust so far, even when used with interactive calls every control step. The issue raised here regards a very specific case where the user sets a very small control step and it leads to very small differences.

[dhblum]

I'm sorry for my delay here. I've started again to study the problem more, also on the bestest_air case. Will report results soon.

[dhblum]

I've benchmarked the performance of bestest_air and bestest_hydronic_heat_pump during the peak heating scenario with constant electricity price for the current master and the proposed changes in #324 across a range of control step sizes.

A few observations to note:

1. The issue is even more pronounced in the bestest_air case.
2. The "Average Results Time Step [s]", which is the average time step of the result trajectories reported by pyfmi, which are returned by the /results API and used by the KPI calculator, is much larger than I anticipated when the control step sizes get long. In the 2wk cases, some individual time steps were up to 2400 s (though I only report the average in the table). Note that in the PR 324 cases, all time steps are just consistently 30 s.
3. The KPI values in the PR 324 cases are significantly more consistent.
4. The "Simulation Time [s]", which is the real time it took to run the full simulation on my computer, is less variable in the PR 324 cases, and lower at smaller control steps.

My thoughts:

1. It is important to have consistency not only on the KPI calculation, but also on the time step of the result trajectories reported to a user who is understanding, evaluating, or using for system ID baseline operation data or data from longer simulation steps. An average time resolution of 1986 s is too long for both. Generating simulation results at a consistent interval helps on both accounts.
2. The usage of the ncp option leads to significantly longer simulation time by pyfmi of each control step (I imagine because there is some additional processing to prepare the data appropriately). Meanwhile, the usage of filter significantly reduces the number of variables pyfmi needs to report and significantly reduces the simulation time of each control step. The overall affect is what is noticed in 4. above. I think a more consistent simulation time is also a good thing. Note, however, that more memory will be required per variable that is reported if the time step is reduced from, say, an average of 1986 s to 30 s.

Seems like implementing a regular time step of results reporting, along with use of the results filter, leads to more consistent performance as a function of control step size on multiple fronts as BOPTEST is currently implemented: KPI values, reporting of data trajectories, and overall simulation time. Adding integrators to the emulators could indeed provide the most consistent KPI results, and is perhaps more robust against further issues I can't yet see. However, it is also an invasive process to the emulator models and/or the BOPTEST infrastructure, and will not address the issues of consistent result reporting time steps and simulation times.

Therefore, I'm wondering if it is a good idea to split some of this thinking/development into two steps. 1) Implement the regular reporting interval for the benefits demonstrated here (discussions of typical data resolution seen in practice aside, which also may differ for each measured variable and sensor type, 30 s seems like a reasonable start to me) and 2) think separately on the necessity and best methods for moving KPI integration within the emulators.

[javiarrobas]

Thanks a lot @dhblum for this elaborate benchmark study. The average result time step is indeed surprisingly large for large control step sizes. I'm glad to see the consistency in KPI results and the lower simulation time variability when using PR 324. Looks like fixing ncp is needed, independently of KPI calculation. I think it's good idea to go as you propose: first implement regular reporting interval, and then think separately whether it's needed to use integration within the emulators.

[dhblum]

@haraldwalnum I wanted to check if you had any more thoughts on this and on my proposed way forward.

[haraldwalnum]

@dhblum, I your proposed way forward is good. In addition to giving more consistent results, I think the fixes in #324 improves the user expericen (due to the reduced computational time). I have actully used the proposed solution in my local version until now. It also solves the memory issues for long time simulation discussed earlier (at least with the emulator complexity we have worked with until now).

I still think energy meters (integrated/accumulative power meters) would be useful. I need it for my controller (at least it works better with that, than i single reading of a power meter), so when I am testing the "bestest hydronic heat pump" emulator I call the result api for each control step and integrate the signal within my controller.

Energy meters could of course be implemented already, independent of KPI calculation, just with a read block. One way to allow them to be used for KPI caclulation, could be to add "energy" to the list of KPI related measurements in the list in the read blocks, as alternative to "power". In the KPI calculation it would then be assumed that measurements marked with "energy" are integrated signals that needs to be differentiated.

[dhblum]

@JavierArroyoBastida and @haraldwalnum, thanks for your feedback on this. Sounds good, let's proceed with #324. Thanks for moving it forward Javi, I'll review.

About energy meters, I think their presence in an emulator, as an alternative to or in addition to power meters, should be decided on an emulator by emulator basis, similar to the presence of other sensors. For use with KPI calculation, your suggestion is true, but are you suggesting a developer could use either "power" or "energy" to contribute to the same energy-related KPI calculations, so that we have a process on the KPI calculation side where for each variable we'd check which is being used and implement the correct calculation process?

[haraldwalnum]

@dhblum, my initial thought was that the emulator developer should choose either "power" or "energy" to contribute to the KPI calculation. The other could still be a measurement, but not connected to KPI calculation (to avoid double counting). The KPI calculation procedure would be chosen based on how the measurement is tagged (power or energy) In the KPI calculation one would just sum energy/emissions/costs calculated from energy meters and from power meters.

[dhblum]

Closed by #324.