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 Adrian Pop, 9 years ago

Component: BackendFrontend
Owner: changed from Lennart Ochel to Adrian Pop
Status: newaccepted

This is first a front-end issue. Then we can switch to the back-end part.

comment:2 by Rüdiger Franke, 9 years ago

Cc: martin.otter@… 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

in reply to:  2 comment:3 by martin.otter@…, 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:4 by Martin Sjölund, 9 years ago

There exists a debug flag for this: -d=evalparam

comment:5 by Rüdiger Franke, 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 Adrian Pop, 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 Martin Sjölund, 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.

comment:8 by Lennart Ochel, 9 years ago

Is it really necessary to evaluate these parameters? I would prefer to move this to the runtime if possible.

in reply to:  8 ; comment:9 by Adrian Pop, 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.

in reply to:  9 comment:10 by Lennart Ochel, 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 Rüdiger Franke, 9 years ago

The current implementation requires the values of integer arguments to sub-clock conversion operators at two places:

  1. SynchronousFeatures.getSubClock -- do rational calculations -- see above example
  2. 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.

Last edited 9 years ago by Rüdiger Franke (previous) (diff)

comment:12 by Rüdiger Franke, 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 Rüdiger Franke, 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 Rüdiger Franke, 9 years ago

Owner: changed from Adrian Pop to Rüdiger Franke
Status: acceptedassigned

comment:15 by Rüdiger Franke, 9 years ago

Resolution: fixed
Status: assignedclosed
Note: See TracTickets for help on using tickets.