Opened 9 years ago
Closed 9 years ago
#3717 closed defect (fixed)
Force evaluation of parameters
Reported by: | Rüdiger Franke | Owned by: | Rüdiger Franke |
---|---|---|---|
Priority: | high | Milestone: | Future |
Component: | Frontend | Version: | v1.9.4-dev-nightly |
Keywords: | Cc: | martin.otter@… |
Description
The current implementation in BackEnd/SynchronousFeatures.mo treats clock settings as rational numbers. This only works for constants or evaluated parameters. In fact Modelica_Synchronous uses the Evaluate=true annotation a lot. Not so in the ShiftSample model. See the following simple example:
model ShiftSample parameter Integer shiftCounter = 1; // annotation(Evaluate=true); parameter Integer resolution = 1; // annotation(Evaluate=true); Real x, y; equation when Clock(1) then x = previous(x) + 1; end when; y = shiftSample(x, shiftCounter, resolution); end ShiftSample;
Translation fails when attempting to get integer constants out of the parameters shiftCounter=e2 and resolution=e3 with:
DAE.ICONST(i1) := e2; DAE.ICONST(i2) := e3;
The model works when uncommenting the Evaluate=true annotations above.
Is there a way to force the evaluation of a parameter, i.e. making it structural during model translation?
Change History (15)
comment:1 by , 9 years ago
Component: | Backend → Frontend |
---|---|
Owner: | changed from | to
Status: | new → accepted |
follow-up: 3 comment:2 by , 9 years ago
Cc: | added |
---|
Is there a reason why many models of the Modelica_Synchronous libraray use Evaluate=true annotations for clock parameters, see e.g.
{Real,Integer,Boolean}Signals.Sampler.{Super,Sub}Sample
whereas other models like
{Real,Integer,Boolean}Signals.Sampler.{Shift,Back}Sample
do not use an Evaluate=true annotations?
See some failing examples in the nightly tests:
https://test.openmodelica.org/libraries/Modelica_Synchronous_cpp/BuildModelRecursive.html
comment:3 by , 9 years ago
Replying to rfranke:
Is there a reason why many models of the Modelica_Synchronous libraray use Evaluate=true annotations for clock parameters, see e.g.
{Real,Integer,Boolean}Signals.Sampler.{Super,Sub}Sample
whereas other models like
{Real,Integer,Boolean}Signals.Sampler.{Shift,Back}Sample
do not use an Evaluate=true annotations?
I assume this was overlooked.
Fixed this (added Evaluate=true) on svn trunk with r9066.
See some failing examples in the nightly tests:
https://test.openmodelica.org/libraries/Modelica_Synchronous_cpp/BuildModelRecursive.html
comment:5 by , 9 years ago
The flag evalparam
evaluates all parameters, which one does not want typically (otherwise it could be the default setting).
The point here is that the current implementation of shiftSample(e1, e2, e3)
in BackEnd/SynchronousFeatures.mo
requires the arguments e2
and e3
to be evaluated, because it uses the values in some rational calculation:
case DAE.CALL(path = Absyn.IDENT("shiftSample"), expLst = {e1, e2, e3}) algorithm (clockKind, (subClock, parentIdx)) := getSubClock1(e1, inVars, inSubClocks); DAE.ICONST(i1) := e2; DAE.ICONST(i2) := e3; subClock.shift := MMath.addRational(subClock.shift, MMath.RATIONAL(i1, i2));
This implementation raises an error if e2
or e3
is a parameter.
How and where can one achieve that the particular arguments e2
and e3
are evaluated if the model does not specify annotation(Evaluate=true)
?
comment:6 by , 9 years ago
We can force this evaluation in the front-end and replace the parameters with constants if possible. For which operators / constructors do we need to do it beside Clock, shiftSample?
@rfranke can you give me a list so I can fix this?
comment:7 by , 9 years ago
Do it in the front-end only if they must be evaluated for all use-cases. Evaluation of things in the frontend makes variables structural, and might make them structural even if constant evaluation fails.
follow-up: 9 comment:8 by , 9 years ago
Is it really necessary to evaluate these parameters? I would prefer to move this to the runtime if possible.
follow-up: 10 comment:9 by , 9 years ago
Replying to lochel:
Is it really necessary to evaluate these parameters? I would prefer to move this to the runtime if possible.
As far as i understood from @rfranke and @bthiele these are structural parameters.
comment:10 by , 9 years ago
Replying to adrpo:
As far as i understood from @rfranke and @bthiele these are structural parameters.
I see! In that case it should be fine to evaluate them.
comment:11 by , 9 years ago
The current implementation requires the values of integer arguments to sub-clock conversion operators at two places:
- SynchronousFeatures.getSubClock -- do rational calculations -- see above example
- SynchronousFeatures.setSubClock (setFactor, setShift) -- compare for equality
The equality must be checked during translation as the model is invalid otherwise.
It affects the integer arguments factor
, shiftCounter
, backCounter
and resolution
of the conversion operators:
subSample(u, factor) superSample(u, factor) shiftSample(u, shiftCounter, resolution) backSample(u, backCounter, resolution)
Btw. also the string valued solverMethod passed to Clock(c, solverMethod)
is treated as sub-clock and checked for equality in setSolverMethod
.
One might argue if the backend should compare parameter expressions for equality instead of comparing rational numbers / strings. Such a comparision would require expression simplification, alias elimination, sorting and the like. I think that a forced evaluation in the frontend could be reverted once this will be needed and implemented.
For the time being the backend fails if the parameters have not been evaluated. Modelica_Synchronous generally uses Evaluate=true
annotations for these parameters, but not consistently because another tool forces the evaluation anyway.
comment:12 by , 9 years ago
The arguments intervalCounter
, resolution
, interval
, startInterval
and solverMethod
to the Clock constructor should also be evaluated.
Clock(intervalCounter, resolution) Clock(interval) Clock(condition, startInterval) Clock(clock, solverMethod)
The current implementation in the backend checks solverMethod
. Moreover it checks intervalCounter
and resolution
in the case a model has multiple base clocks. interval
should also be checked in that case. See e.g. Modelica_Synchronous.Examples.CascadeControlledDrive.AbsoluteClocks
.
Note: Modelica supports inferred clocks if the determination of specific clock intervals shall be deferred to the runtime.
comment:13 by , 9 years ago
Forget my previous comment:12. Base clocks are generally not structural, meaning that only the Clock argument solverMethod
should be evaluated in the frontend, besides the sub-clock conversion parameters given in comment:11.
This implies that we allow the backend to fail if a model has multiple base clocks in one clocked partition, see e.g. Modelica_Synchronous.Examples.CascadeControlledDrive.AbsoluteClocks
, and does not specify annotation(Evaluate=true)
. I think that multiple base clocks in one clocked partition are bad style. One should use sub-clock conversions. See e.g. the alternative formulations CascadeControlledDrive.{SubClocked,SuperSampled}
.
It is more important to support the following model snippet stated in the Modelica spec:
Integer nextInterval(start=2); // first interval = 2/100 equation when Clock(nextInterval, 100) then // interval clock that ticks at 0, 0.02, 0.05, 0.09, ... nextInterval = previous(nextInterval) + 1; y = previous(y) + 1; end when;
comment:14 by , 9 years ago
Owner: | changed from | to
---|---|
Status: | accepted → assigned |
comment:15 by , 9 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
This is first a front-end issue. Then we can switch to the back-end part.