TestCase: Bestest Hydronic pump control

[haraldwalnum]

@dhblum @JavierArroyoBastida . As discussed yesterday I had some issues in controlling the pump on/off in the besttest hydronic testcase. I have tried to reproduce the issue in a simple test setup. I run the emulator with 3600s control step, turning the pump on/off for each step. The results are show below. One can see that ovePump_u and reaPump_y follows the control signal, but reaPPum_y consumes power more "randomly". This is also reflected by the gas consumed by the heater (reaQHea_y).

I have also attached the code I used to run the test.
pumpTest.txt

[javiarrobas]

Thanks @haraldwalnum for sharing the issue and script, I could easily reproduce the problem. I've been doing some testing on this and it's indeed an awkward behavior. I'm afraid I've not been able to realize the source of the problem, but have two comments on this:

1. Not sure if we're checking out the right commit of IDEAS for this testcase: we're using now a91fd24d97a9dff737501a94cb6387530d618f6c but I couldn't find that commit in the IDEAS library... not sure where it's coming from, maybe it's just me but just couldn't find it. I think we should check out: f90badebc912251714a4ca93c86739cca34ea044. Still, nothing has changed when checking out the new commit.
2. I have extended the model to have a "debug model" that reads more variables which might help us to figure out what's going on. Basically, I've added read blocks for the integer that sets the pump stage, the pressure difference in the pump and the normalized pump speed. This debug model looks as follows:

See results here (also with indoor temperature):

What is interesting from here is that the integer variable that sets the pump stage is already corrupted, which indicates that the problem comes from the realToInteger. The doc of this model:

Where you can see that there is not definition of the output value when u=0, which I thought could be the origin of the problem. I changed the model in this branch of ideas: https://github.com/open-ideas/IDEAS/tree/issue1191_boptestBestestControl, to use a boolean threshold instead. Looking like:

but not improving results:

I'll keep trying when time allows.
I'm also adding @Mathadon in the loop since this relates to open-ideas/IDEAS#1191.

[javiarrobas]

I have increased the boolean threshold in greThr to be 0.1 instead of 0, just in case the problem could come from any kind of computational noise. Still the exact same result.

[javiarrobas]

Changing the pump input to be continuous instead of integer to select stage seems to solve the problem. See model:

and output from @haraldwalnum's script now:

I see this as a fair solution, although there are some things to take into account:

1. User would be able to modulate the pump. Not sure whether that's an advantage or a disadvantage. It relates to our dilemma on what can be measured/controlled in the emulators.
2. Other emulators should be revised like the bestest_hydronic_heat_pump which also uses integers to control pump stage.
3. The most important one: it is annoying not to know what was causing the problem!

[dhblum]

Thanks @haraldwalnum for reporting and @JavierArroyoBastida for looking into it. Also, indeed putting a first order filter before the real-to-integer block helps too (see results below from Harald's script). FYI Applying the filter after the real-to-integer block does not help, including using the filter within the pump itself.

[dhblum]

My feeling is that it has to do with triggering event updates at the start of a step related to the real-to-integer block. The filter forces a state event and using the continuous signal removes the real-to-integer block altogether. See also #213 for relation.

[javiarrobas]

Thanks @dhblum, that helps me understand a bit better.
Still, we need to make a decision on how to solve this. We can either use the filter or the continuous actions. Do you have any preference? or maybe another suggestion to try out?

[haraldwalnum]

In my opinion, the filter solution is the best. It solves point 1 of Javier. However, point 2 is still valid an I guess it can occur in all emulators using real to integer blocks or hysteresis (which is common in controllers). Maybe also in other cases?

[SenHuang19]

a step function is generally approximated as a ramping function in JModelica. So whether the event is triggered or not depends on how the step function is interpolated, which is associated with the simulation interval the solver selects. Adding a filter basically defines how the step function should be interpolated but my concern is that it may add artificial delays

[dhblum]

I've been spending some time trying to get even further to the root of the issue. Specifically, why potential events aren't checked for at the start of a simulation step, when that simulation step could have changed inputs from the last one? So I've gone digging through the pyfmi code.

In BOPTEST's testcase.py, simulation at each step occurs using the fmu.simulate() function, which is a high-level function that wraps a number of FMI functions together depending on whether the FMU is a ME or CS. In our case, we have an ME. So the fmu.simulate() function instantiates an instance of class AssimuloFMIAlg from fmi_algorithm_drivers.py, uses it to prepare the model for a simulation step given inputs and options, and then takes the simulation step by using Assimulo's CVODE solver and associated interface in simulation/assimulo_interface.py. Anyway, in fmi_algorithm_drivers.py line ~338, there is the following code:

        #See if there is an time event at start time
        if isinstance(self.model, fmi.FMUModelME1):
            event_info = self.model.get_event_info()
            if event_info.upcomingTimeEvent and event_info.nextEventTime == model.time:
                self.model.event_update()

My inclination seems right, that in the preparation for the simulation step, there should be a check of an event at start time (which would occur if the input changed values from the last step). But, notice that the if clause only does so for an ME1. It skips this check altogether for ME2.

So, I did a test where upon building the testcase docker image I add the following to fmi_algorithm_drivers.py after the above code, to basically do the same for ME 2:

        if isinstance(self.model, fmi.FMUModelME2):
            self.model.event_update()         

Then, I compiled the testcase (original, with the real-to-integrator block and without the filter), ran Harald's script, and BINGO:

EDIT:

• Here's the line in current pyfmi github: https://github.com/modelon-community/PyFMI/blob/master/src/pyfmi/fmi_algorithm_drivers.py#L345. Potentially a bug not to check for ME2 as well?
• Here's a branch with the updated Dockerfile for the code change if anyone else wants to try: https://github.com/ibpsa/project1-boptest/tree/issue274_hydronicPumpControl

[javiarrobas]

Wow @dhblum this is great, thanks for taking the time to figure it out.
Does this mean that we can just use the new Dockerfile with the pyfmi edit?
It looks like a pyfmi bug to me. Do you Know by any chance what's the difference between FMU1 and FMU2? From the source-code definition I see:

FMUModelME1: An FMI Model loaded from a DLL. (see here)
FMUModelME2: Model-exchange model loaded from a dll. (see here)

But that doesn't say much to me.

[dhblum]

@JavierArroyoBastida Yes, I think the new Dockerfile should be sufficient. But I'd like to repeat the small test I did in #213 to double check on that case, and I think also report an issue with pyfmi development to see what they say. And maybe @haraldwalnum wants to try his previous test case that ran into problems that prompted that previous issue.

The 1 and the 2 specify which version of the FMI standard is supported.

[dhblum]

Another update. Indeed based on a simpler tests the fix described above helps. Consider the model:

Compiling into an ME2 FMU and simulating in three steps of 100 each with changes in input values in each of u1,u2,u3 at the start of each step yields the following:

[haraldwalnum]

Thanks again @dhblum . This fix works on my test case as well.

[dhblum]

@haraldwalnum Great! I've reported this issue to the pyfmi repo.

If a fix is indeed necessary, it will be made in a future version of pyfmi. Currently, the BOPTEST Docker uses the last trunk of JModelica.org and the pyfmi version included therein. There's already an incomplete issue #146 to migrate this to a stand-alone pyfmi install, where we could then utilize any new version of pyfmi that would contain any fix. But there were some challenges to migrating to a stand-alone pyfmi install that needed to be resolved. I've actually made some good progress on this but haven't gotten a chance to formalize and report back to that issue. Will aim to do so soon.

In the meantime of hearing back from pyfmi developers and migrating the BOPTEST container to a stand-alone pyfmi install, I suggest using the patch I propose in the Dockerfile here https://github.com/ibpsa/project1-boptest/tree/issue274_hydronicPumpControl. Any thoughts on this?

[dhblum]

EDIT: I referenced the wrong pyfmi issue in the above comment. It is actually: modelon-community/PyFMI#72.

[haraldwalnum]

Sounds good to me.

[dhblum]

Closed by #284.