Closed AntoineGautier closed 2 months ago
The block TrueHold
can indeed be simplified, which I started on issue3787_trueHold_latch
I don't yet see why the difference in lat.y
and lat1.y
occurs. Could it be due to rounding errors?
I submitted the model LatchWithHoldAlt
included below to DS support (ticket #SR01185379).
It reproduces the issue observed in LatchWithHold
above but without any dependencies to Buildings.
[EDIT on 4/19/24] This seems to be considered as a bug by DS. I am opening a ticket at Modelon: id #876
.
[EDIT on 4/25/24] Ticket #876
open with Modelon support, currently handled by the compiler team.
@mwetter @JayHuLBL The block LatchSignal
included in LatchWithHoldAlt
below is an alternative implementation of the Latch
block from MBL. However, I cannot see why this simplified implementation does not fully represent the intent of the Latch
block as described in the documentation. At least, it gives the same results for the validation model Buildings.Controls.OBC.CDL.Logical.Validation.Latch
. Please let me know if I am missing something or if the current implementation could be replaced with this one.
model LatchWithHoldAlt
block LatchSignal "Maintain a true signal until cleared"
Modelica.Blocks.Interfaces.BooleanInput u
"Latch input"
annotation (Placement(transformation(extent={{-140,-20},{-100,20}})));
Modelica.Blocks.Interfaces.BooleanInput clr
"Clear input"
annotation (Placement(transformation(extent={{-140,-80},{-100,-40}})));
Modelica.Blocks.Interfaces.BooleanOutput y
"Output signal"
annotation (Placement(transformation(extent={{100,-20},{140,20}})));
equation
when initial() then
y=not clr and u;
elsewhen {clr, u} then
y=not clr and u;
end when;
end LatchSignal;
Modelica.Blocks.Sources.BooleanPulse booleanPulse(
width=20,
period=1,
startTime=0.5)
annotation (Placement(transformation(extent={{-80,-10},{-60,10}})));
LatchSignal lat1
annotation (Placement(transformation(extent={{60,-70},{80,-50}})));
Modelica.Blocks.Logical.Not clr1
annotation (Placement(transformation(extent={{10,-90},{30,-70}})));
LatchSignal lat
annotation (Placement(transformation(extent={{60,-10},{80,10}})));
Modelica.Blocks.Logical.Not clr
annotation (Placement(transformation(extent={{10,-30},{30,-10}})));
inner Modelica.StateGraph.StateGraphRoot stateGraphRoot
"Root of state graph"
annotation (Placement(transformation(extent={{70,68},{90,88}})));
Modelica.StateGraph.InitialStepWithSignal initialStepWithSignal(
nIn=1,
nOut=1)
"Initial step"
annotation (Placement(transformation(extent={{-80,50},{-60,70}})));
Modelica.StateGraph.StepWithSignal outputTrue(
nIn=1,
nOut=1)
"Holds the output at true"
annotation (Placement(transformation(extent={{-10,50},{10,70}})));
Modelica.StateGraph.TransitionWithSignal toOutputTrue
"Transition that activates sending a true output signal"
annotation (Placement(transformation(extent={{-50,50},{-30,70}})));
Modelica.StateGraph.Transition toInitial(
enableTimer=true,
waitTime=0.2)
"Transition that activates the initial state"
annotation (Placement(transformation(extent={{30,50},{50,70}})));
equation
connect(booleanPulse.y, clr1.u)
annotation (Line(points={{-59,0},{-40,0},{-40,-80},{8,-80}},color={255,0,255}));
connect(clr1.y, lat1.clr)
annotation (Line(points={{32,-80},{40,-80},{40,-66},{58,-66}},color={255,0,255}));
connect(booleanPulse.y, lat1.u)
annotation (Line(points={{-59,0},{-40,0},{-40,-60},{58,-60}},color={255,0,255}));
connect(initialStepWithSignal.outPort[1], toOutputTrue.inPort)
annotation (Line(points={{-59.5,60},{-44,60}},color={0,0,0}));
connect(toInitial.outPort, initialStepWithSignal.inPort[1])
annotation (Line(points={{41.5,60},{60,60},{60,80},{-90,80},{-90,60},{-81,60}},
color={0,0,0}));
connect(toOutputTrue.outPort, outputTrue.inPort[1])
annotation (Line(points={{-38.5,60},{-11,60}},color={0,0,0}));
connect(outputTrue.outPort[1], toInitial.inPort)
annotation (Line(points={{10.5,60},{36,60}},color={0,0,0}));
connect(booleanPulse.y, toOutputTrue.condition)
annotation (Line(points={{-59,0},{-40,0},{-40,48}},color={255,0,255}));
connect(booleanPulse.y, lat.u)
annotation (Line(points={{-59,0},{58,0}},color={255,0,255}));
connect(outputTrue.active, clr.u)
annotation (Line(points={{0,49},{0,-20},{8,-20}},color={255,0,255}));
connect(clr.y, lat.clr)
annotation (Line(points={{31,-20},{40,-20},{40,-6},{58,-6}},color={255,0,255}));
annotation (
experiment(
StopTime=1.0,
Tolerance=1e-06));
end LatchWithHoldAlt;
@AntoineGautier I think the simplified Latch
implementation LatchSignal
can replace the current implementation. It indeed has the same results.
After discussing with Robin Andersson at Modelon, the results are as expected.
This is because two steps of a state machine that are connected with a transition cannot be active simultaneously. This leads to two distinct event iterations at the same time instant (0.5 s in the example). During the first one, the transition becomes active and the upstream state is still active. During the second one only, the downstream state becomes active (and both the upstream state and the transition become inactive). So when the clause elsewhen {clr, u}
is true, not clr
is still false and lat1.y
remains false.
I can see two ways moving forward with this issue and I suggest implementing the two. (Each one individually yields the expected results when simulating LatchWithHold
.)
In Controls.OBC.CDL.Logical.Latch
: Change the clause elsewhen {clr, u}
into elsewhen {change(clr), u}
.
This changes the intent of the block as described in point 2) at https://github.com/lbl-srg/modelica-buildings/issues/3796#issuecomment-2051927205.
However, if a Dirac pulse is used for the clr
signal (see Jianjun's comment), then the output does switch to false when using elsewhen {change(clr), u}
— which was not the case when using elsewhen {clr, not clr, u}
as proposed in point 2) above.
In Controls.OBC.CDL.Logical.TrueHold
: Conditionally remove all state graph components if duration==0
and use a direct pass through of the input signal so that y=u
.
@mwetter @JayHuLBL What do you think?
I think these changes are fine. @JayHuLBL , do you agree?
The change looks good to me.
This in fact may also address issue #3841, which would relate to the block TrueFalseHold
. We would need to conditionally remove the corresponding state graph components if the trueHoldDuration
or falseHoldDuration
is zero. I will look into it.
Updates after trying to implement the changes proposed above:
In Controls.OBC.CDL.Logical.Latch
: Changing the clause elsewhen {clr, u}
into elsewhen {change(clr), u}
yields a behavior that I eventually find very tedious to describe in English. The issue is that, in a scenario with a constant true input,
clr
signal clears the output which remains false until the input transitions back to true, butclr
signal that transitions to true and then to false first clears the output then releases it back to true.So I find this even more counter-intuitive than what I described at https://github.com/lbl-srg/modelica-buildings/issues/3796#issuecomment-2051927205. And I tend to finally agree with Jianjun's initial comment suggesting that the clause elsewhen {clr, u}
should remain as is.
In Controls.OBC.CDL.Logical.TrueHold
:
i * duration
where i
is the smallest integer such that i * duration >= time(u switches to false) - time(hold started)
. This behavior aligns with the logic described in the documentation. However, this differs from the implementation of Controls.OBC.CDL.Logical.TrueFalseHold
where the output is true for the longer of trueHoldDuration
and the time during which the input signal is true. This latter logic matches the common pattern used in control sequences and aligns with the intuitive understanding of what "hold for a given duration" means. Controls.OBC.CDL.Logical.TrueHold
loops over the state graph under the rationale that "the transitions are done in zero time" (as described in the documentation). This is brittle because the output is not strictly held constant, i.e., there is an event iteration during this "zero time" transition where it switches to false. This can be verified by simulating the model TrueFalseHoldTrueHold
below. In this model, the instances TrueHold
and TrueFalseHold
appear to generate the same output that is constantly true between 0 and 1 s. But comparing the trajectories lat.y
and lat1.y
shows that truHol.y
does indeed transition to false at 0.5 s whereas truFalHol.y
does not.TrueHold
to adopt the same logic and use the same implementation as TrueFalseHold
: thus, the output will be held for the longer of duration
and the time during which the input signal is true. I also propose to refactor TrueFalseHold
with direct pass-through if the duration is zero: this will keep the two blocks consistent. model TrueFalseHoldTrueHold
Buildings.Controls.OBC.CDL.Logical.TrueHold truHol(duration=0.5)
annotation (Placement(transformation(extent={{-40,-10},{-20,10}})));
Buildings.Controls.OBC.CDL.Logical.Sources.Constant con(k=true)
annotation (Placement(transformation(extent={{-90,-10},{-70,10}})));
Buildings.Controls.OBC.CDL.Logical.Latch lat
annotation (Placement(transformation(extent={{60,-10},{80,10}})));
Buildings.Controls.OBC.CDL.Logical.Sources.Pulse booPul1(
width=0.5,
period=2,
shift=0.1) annotation (Placement(transformation(extent={{0,30},{20,50}})));
Buildings.Controls.OBC.CDL.Logical.Not not1
annotation (Placement(transformation(extent={{0,-10},{20,10}})));
Buildings.Controls.OBC.CDL.Logical.TrueFalseHold truFalHol(trueHoldDuration=0.5,
falseHoldDuration=0)
annotation (Placement(transformation(extent={{-40,-50},{-20,-30}})));
Buildings.Controls.OBC.CDL.Logical.Not not2
annotation (Placement(transformation(extent={{0,-50},{20,-30}})));
Buildings.Controls.OBC.CDL.Logical.Latch lat1
annotation (Placement(transformation(extent={{60,-50},{80,-30}})));
equation
connect(con.y, truHol.u)
annotation (Line(points={{-68,0},{-42,0}}, color={255,0,255}));
connect(booPul1.y, lat.u) annotation (Line(points={{22,40},{50,40},{50,0},{58,
0}}, color={255,0,255}));
connect(not1.y, lat.clr) annotation (Line(points={{22,0},{40,0},{40,-6},{58,-6}},
color={255,0,255}));
connect(truHol.y, not1.u)
annotation (Line(points={{-18,0},{-2,0}}, color={255,0,255}));
connect(con.y, truFalHol.u) annotation (Line(points={{-68,0},{-60,0},{-60,-40},
{-42,-40}}, color={255,0,255}));
connect(truFalHol.y, not2.u)
annotation (Line(points={{-18,-40},{-2,-40}}, color={255,0,255}));
connect(booPul1.y, lat1.u) annotation (Line(points={{22,40},{50,40},{50,-40},{
58,-40}}, color={255,0,255}));
connect(not2.y, lat1.clr) annotation (Line(points={{22,-40},{40,-40},{40,-46},
{58,-46}}, color={255,0,255}));
annotation (uses(Buildings(version="12.0.0")));
end TrueFalseHoldTrueHold;
@AntoineGautier Thanks for the detailed information.
For the latch
block, keep elsewhen {clr,u}
rather than elsewhen{change(clr),u}
.
For the TrueFalseHold
, I am actually updating the block as suggested: conditional remove the state graphical component when the duration is zero. Please see branch issue3841_trueFalseHold
.
After discussing with @mwetter and @JayHuLBL the plan is now to:
TrueHold
with TrueFalseHold(trueHoldDuration=%duration%, falseHoldDuration=0)
and move TrueHold
to Obsolete
.TrueFalseHold
to use direct pass-throughs if the duration is zero.This is done in issue3787_latchAndHold
.
Refactoring TrueFalseHold
with direct pass-throughs if the duration is zero did not solve the initial issue. This is because the unexpected behavior — due to "hidden" event iterations — may not only occur when the duration is zero, but potentially at every instant when the variables u
and y
change simultaneously.
This is illustrated in the example TrueFalseHoldAndLatch
below where each instance of the Latch
block is expected to produce the same output as the reference case latRef
. I was not able to reach this objective by refactoring the current implementation with state graphs. I believe that it would significantly increase the block's complexity, which contradicts the initial purpose of using state graphs.
Therefore, I refactored the block using elementary synchronous language constructs. This is done in 6a1692b6de, and the implementation has been further simplified in 085d42744e2640711306039671f1c8b4c3517dd0.
model TrueFalseHoldAndLatch
Buildings.Controls.OBC.CDL.Logical.Latch latRef
"Latch without state graph logic – Reference case"
annotation (Placement(transformation(extent={{-50,70},{-30,90}})));
Buildings.Controls.OBC.CDL.Logical.TrueFalseHold truFalHol(trueHoldDuration=
0.2, falseHoldDuration=0.2) "True and false hold"
annotation (Placement(transformation(extent={{-130,-110},{-110,-90}})));
Buildings.Controls.OBC.CDL.Logical.Sources.Pulse booPul(
width=0.2,
period=1,
shift=0.5) "Source signal"
annotation (Placement(transformation(extent={{-180,-10},{-160,10}})));
Buildings.Controls.OBC.CDL.Logical.Not clrTruHol "Compute clear signal"
annotation (Placement(transformation(extent={{-88,-110},{-68,-90}})));
Buildings.Controls.OBC.CDL.Logical.Not clrRef "Compute clear signal"
annotation (Placement(transformation(extent={{-130,50},{-110,70}})));
Buildings.Controls.OBC.CDL.Logical.Latch latTru0FalHol
"Latch with clear signal computed with zero true hold duration"
annotation (Placement(transformation(extent={{-50,-10},{-30,10}})));
Buildings.Controls.OBC.CDL.Logical.TrueFalseHold tru0FalHol(trueHoldDuration=
0, falseHoldDuration=0.2) "Zero true hold duration"
annotation (Placement(transformation(extent={{-130,-30},{-110,-10}})));
Buildings.Controls.OBC.CDL.Logical.Not clrTru0FalHol "Compute clear signal"
annotation (Placement(transformation(extent={{-90,-30},{-70,-10}})));
Buildings.Controls.OBC.CDL.Logical.Latch latTruHol
"Latch with clear signal computed with true and false hold"
annotation (Placement(transformation(extent={{-48,-90},{-28,-70}})));
Buildings.Controls.OBC.CDL.Logical.TrueFalseHold tru0Fal0Hol(trueHoldDuration
=0, falseHoldDuration=0) "Zero true and false hold duration"
annotation (Placement(transformation(extent={{-130,10},{-110,30}})));
Buildings.Controls.OBC.CDL.Logical.Not clrTru0Fal0Hol "Compute clear signal"
annotation (Placement(transformation(extent={{-90,10},{-70,30}})));
Buildings.Controls.OBC.CDL.Logical.Latch latTru0Fal0Hol
"Latch with clear signal computed with zero true and false hold duration"
annotation (Placement(transformation(extent={{-50,30},{-30,50}})));
Buildings.Controls.OBC.CDL.Logical.Latch latTruFal0Hol
"Latch with clear signal computed with zero false hold duration"
annotation (Placement(transformation(extent={{-48,-50},{-28,-30}})));
Buildings.Controls.OBC.CDL.Logical.TrueFalseHold truFal0Hol(trueHoldDuration=
0.2, falseHoldDuration=0) "Zero false hold duration"
annotation (Placement(transformation(extent={{-130,-70},{-110,-50}})));
Buildings.Controls.OBC.CDL.Logical.Not clrTruFal0Hol "Compute clear signal"
annotation (Placement(transformation(extent={{-88,-70},{-68,-50}})));
Buildings.Controls.OBC.CDL.Logical.TrueFalseHold truFalHol1(trueHoldDuration=
0.2, falseHoldDuration=0.2) "True and false hold"
annotation (Placement(transformation(extent={{50,-110},{70,-90}})));
Buildings.Controls.OBC.CDL.Logical.Not clrTruHol1 "Compute clear signal"
annotation (Placement(transformation(extent={{10,-110},{30,-90}})));
Buildings.Controls.OBC.CDL.Logical.Latch latTru0FalHol1
"Latch with clear signal computed with zero true hold duration"
annotation (Placement(transformation(extent={{90,-10},{110,10}})));
Buildings.Controls.OBC.CDL.Logical.TrueFalseHold tru0FalHol1(trueHoldDuration
=0, falseHoldDuration=0.2) "Zero true hold duration"
annotation (Placement(transformation(extent={{50,-30},{70,-10}})));
Buildings.Controls.OBC.CDL.Logical.Not clrTru0FalHol1 "Compute clear signal"
annotation (Placement(transformation(extent={{10,-30},{30,-10}})));
Buildings.Controls.OBC.CDL.Logical.Latch latTruHol1
"Latch with clear signal computed with true and false hold"
annotation (Placement(transformation(extent={{90,-90},{110,-70}})));
Buildings.Controls.OBC.CDL.Logical.TrueFalseHold tru0Fal0Hol1(
trueHoldDuration=0, falseHoldDuration=0)
"Zero true and false hold duration"
annotation (Placement(transformation(extent={{50,10},{70,30}})));
Buildings.Controls.OBC.CDL.Logical.Not clrTru0Fal0Hol1 "Compute clear signal"
annotation (Placement(transformation(extent={{10,10},{30,30}})));
Buildings.Controls.OBC.CDL.Logical.Latch latTru0Fal0Hol1
"Latch with clear signal computed with zero true and false hold duration"
annotation (Placement(transformation(extent={{90,30},{110,50}})));
Buildings.Controls.OBC.CDL.Logical.Latch latTruFal0Hol1
"Latch with clear signal computed with zero false hold duration"
annotation (Placement(transformation(extent={{90,-50},{110,-30}})));
Buildings.Controls.OBC.CDL.Logical.TrueFalseHold truFal0Hol1(trueHoldDuration
=0.2, falseHoldDuration=0) "Zero false hold duration"
annotation (Placement(transformation(extent={{50,-70},{70,-50}})));
Buildings.Controls.OBC.CDL.Logical.Not clrTruFal0Hol1 "Compute clear signal"
annotation (Placement(transformation(extent={{10,-70},{30,-50}})));
equation
connect(truFalHol.y, clrTruHol.u)
annotation (Line(points={{-108,-100},{-90,-100}}, color={255,0,255}));
connect(booPul.y, truFalHol.u) annotation (Line(points={{-158,0},{-140,0},{
-140,-100},{-132,-100}}, color={255,0,255}));
connect(booPul.y, clrRef.u) annotation (Line(points={{-158,0},{-140,0},{-140,
60},{-132,60}}, color={255,0,255}));
connect(booPul.y, latRef.u) annotation (Line(points={{-158,0},{-140,0},{-140,
80},{-52,80}}, color={255,0,255}));
connect(clrRef.y, latRef.clr) annotation (Line(points={{-108,60},{-80,60},{
-80,74},{-52,74}}, color={255,0,255}));
connect(booPul.y, latTru0Fal0Hol.u) annotation (Line(points={{-158,0},{-140,0},
{-140,40},{-52,40}}, color={255,0,255}));
connect(tru0Fal0Hol.y, clrTru0Fal0Hol.u)
annotation (Line(points={{-108,20},{-92,20}}, color={255,0,255}));
connect(clrTru0Fal0Hol.y, latTru0Fal0Hol.clr) annotation (Line(points={{-68,
20},{-60,20},{-60,34},{-52,34}}, color={255,0,255}));
connect(booPul.y, tru0Fal0Hol.u) annotation (Line(points={{-158,0},{-140,0},{
-140,20},{-132,20}}, color={255,0,255}));
connect(booPul.y, tru0FalHol.u) annotation (Line(points={{-158,0},{-140,0},{
-140,-20},{-132,-20}}, color={255,0,255}));
connect(tru0FalHol.y, clrTru0FalHol.u)
annotation (Line(points={{-108,-20},{-92,-20}}, color={255,0,255}));
connect(clrTru0FalHol.y, latTru0FalHol.clr) annotation (Line(points={{-68,-20},
{-60,-20},{-60,-6},{-52,-6}}, color={255,0,255}));
connect(booPul.y, latTruFal0Hol.u) annotation (Line(points={{-158,0},{-140,0},
{-140,-40},{-50,-40}}, color={255,0,255}));
connect(truFal0Hol.y, clrTruFal0Hol.u)
annotation (Line(points={{-108,-60},{-90,-60}}, color={255,0,255}));
connect(clrTruFal0Hol.y, latTruFal0Hol.clr) annotation (Line(points={{-66,-60},
{-60,-60},{-60,-46},{-50,-46}}, color={255,0,255}));
connect(booPul.y, truFal0Hol.u) annotation (Line(points={{-158,0},{-140,0},{
-140,-60},{-132,-60}}, color={255,0,255}));
connect(clrTruHol.y, latTruHol.clr) annotation (Line(points={{-66,-100},{-60,
-100},{-60,-86},{-50,-86}}, color={255,0,255}));
connect(booPul.y, latTruHol.u) annotation (Line(points={{-158,0},{-140,0},{
-140,-80},{-50,-80}}, color={255,0,255}));
connect(booPul.y, latTru0FalHol.u)
annotation (Line(points={{-158,0},{-52,0}}, color={255,0,255}));
connect(booPul.y, latTru0Fal0Hol1.u) annotation (Line(points={{-158,0},{0,0},
{0,40},{88,40}}, color={255,0,255}));
connect(booPul.y, latTruFal0Hol1.u) annotation (Line(points={{-158,0},{0,0},{
0,-40},{88,-40}}, color={255,0,255}));
connect(booPul.y, latTruHol1.u) annotation (Line(points={{-158,0},{0,0},{0,
-80},{88,-80}}, color={255,0,255}));
connect(booPul.y, latTru0FalHol1.u)
annotation (Line(points={{-158,0},{88,0}}, color={255,0,255}));
connect(clrTru0Fal0Hol1.y, tru0Fal0Hol1.u)
annotation (Line(points={{32,20},{48,20}}, color={255,0,255}));
connect(tru0Fal0Hol1.y, latTru0Fal0Hol1.clr) annotation (Line(points={{72,20},
{79,20},{79,34},{88,34}}, color={255,0,255}));
connect(booPul.y, clrTru0Fal0Hol1.u) annotation (Line(points={{-158,0},{0,0},
{0,20},{8,20}}, color={255,0,255}));
connect(truFalHol1.y, latTruHol1.clr) annotation (Line(points={{72,-100},{80,
-100},{80,-86},{88,-86}}, color={255,0,255}));
connect(truFal0Hol1.y, latTruFal0Hol1.clr) annotation (Line(points={{72,-60},
{80,-60},{80,-46},{88,-46}}, color={255,0,255}));
connect(tru0FalHol1.y, latTru0FalHol1.clr) annotation (Line(points={{72,-20},
{80,-20},{80,-6},{88,-6}}, color={255,0,255}));
connect(clrTru0FalHol1.y, tru0FalHol1.u)
annotation (Line(points={{32,-20},{48,-20}}, color={255,0,255}));
connect(truFal0Hol1.u, clrTruFal0Hol1.y)
annotation (Line(points={{48,-60},{32,-60}}, color={255,0,255}));
connect(clrTruHol1.y, truFalHol1.u)
annotation (Line(points={{32,-100},{48,-100}}, color={255,0,255}));
connect(booPul.y, clrTru0FalHol1.u) annotation (Line(points={{-158,0},{0,0},{
0,-20},{8,-20}}, color={255,0,255}));
connect(booPul.y, clrTruFal0Hol1.u) annotation (Line(points={{-158,0},{0,0},{
0,-60},{8,-60}}, color={255,0,255}));
connect(booPul.y, clrTruHol1.u) annotation (Line(points={{-158,0},{0,0},{0,
-100},{8,-100}}, color={255,0,255}));
annotation (Icon(coordinateSystem(preserveAspectRatio=false)), Diagram(
coordinateSystem(preserveAspectRatio=false, extent={{-200,-200},{200,
200}})));
end TrueFalseHoldAndLatch;
When simulating the model
LatchWithHold
included below, the blockslat
andlat1
have the same input values but different output values.The only difference is that
lat.clr
is computed using the blockTrueHold
whereaslat1.clr
is not. The blocklat1
generates the expected output based on the documentation ofLatch
. Both Dymola and OCT produce the same results. Perhaps the implementation ofTrueHold
using a state graph is the cause although I can't find an explanation for this in the language specification.On a side note, I wonder whether the implementation of
Latch
could not be simplified by removing the componentonDelay
and usingModelica.StateGraph.Transition toInitial(enableTimer=duration > 0, waitTime=duration)
.