Strange tuple in initial equation in Modelica.Blocks.Examples.Filter

[Peter Aronsson]

Modelica.Blocks.Examples.Filter returns this equation in the flat model:

(CriticalDamping.r, {}, {}, {}) = Modelica.Blocks.Continuous.Internal.Filter.roots.lowPass({CriticalDamping.cr[1], CriticalDamping.cr[2], CriticalDamping.cr[3]}, {}, {}, CriticalDamping.f_cut);

Shouldn't this instead be:

(CriticalDamping.r, , , ) = Modelica.Blocks.Continuous.Internal.Filter.roots.lowPass({CriticalDamping.cr[1], CriticalDamping.cr[2], CriticalDamping.cr[3]}, {}, {}, CriticalDamping.f_cut);

[Adrian Pop]

I'll have a look.

[Adrian Pop]

Is not strange.
It comes from equation:

        if filterType == Modelica.Blocks.Types.FilterType.LowPass then
          (r, a, b, ku) = Internal.Filter.roots.lowPass(cr, c0, c1, f_cut);

and for component:

        Modelica.Blocks.Continuous.Filter CriticalDamping(
          analogFilter = Modelica.Blocks.Types.AnalogFilter.CriticalDamping, 
          normalized = normalized, 
          init = init, filterType = filterType, 
          order = order, 
          f_cut = f_cut, 
          f_min = 0.8 * f_cut);

filterType is LowPass which means that equation will be selected.

components a, b, ku are arrays of 0 size, which mean they disappear from the DAE.

        parameter Real[na] a(each fixed = false);
        parameter Real[na] b(each fixed = false);
        parameter Real[na] ku(each fixed = false);

I don't know if it would be better to put them as empty instead of {}.

[Peter Aronsson]

Yes, I realized that when I wrote the ticket, but wouldn't it be nicer from a backend perspective if those were rewritten to "empty slots" instead, which is standard Modelica? That way, the backend doesn't have to deal with all possible empty arrays that could be in there, e.g. {}, {{}}, . And the backend still needs to take care of the case (a_{) = foo(b,c), so it would simplify the backend alot.}

This is not supercritical for us right now, since I've made a workaround for now, but all attempts at simplifying stuff are good, I think ;-)

[Adrian Pop]

I think that we should actually keep all zero size components and zero size equations/algorithms in the DAE and they can be removed by the back-end.
What do you think about this?

[Martin Sjölund]

Postponed until 1.9.1

[Martin Sjölund]

It's a very small diff to change this, but our backend cannot handle it:

Compiler/FrontEnd/InstSection.mo

@@ -1377 +1377 @@
 algorithm
   outDae := matchcontinue (inExp1,inExp2,inType3, inConst, source,inInitial4)
     local
-      DAE.DAElist dae; DAE.Exp e1,e2;
+      DAE.DAElist dae;
+      DAE.Exp e,e1,e2;
       SCode.Initial initial_;
       DAE.ComponentRef cr;
       DAE.Type t;
@@ -1385 +1386 @@
       DAE.Type tt;
       list<DAE.Exp> exps1,exps2;
       list<DAE.Type> tys;
+      Boolean b;
 
     case (e1,e2,DAE.T_INTEGER(varLst = _),_,_,initial_)
       equation
@@ -1420 +1422 @@
       then dae;
 
     // tuples
+    case (DAE.TUPLE(e::_),e2,DAE.T_TUPLE(tupleType = t::tys),_,_,initial_)
+      equation
+        b = List.fold(List.map(tys,Types.isEmptyArray),boolAnd,true);
+        e1 = Util.if_(b,e,inExp1);
+        // e2 = Util.if_(b,DAE.TSUB(inExp2,1,t),inExp2);
+        dae = makeDaeEquation(e1, e2, source, initial_);
+      then
+        dae;
     case (e1,e2,DAE.T_TUPLE(tupleType = _),_,_,initial_)
       equation
         dae = makeDaeEquation(e1, e2, source, initial_);

(The result is the equation is split into 3, but without ASUB. So it is slower, and even if it worked, it would be slower.)

I am reassigning this to backend, to see if we can change the flat code.

[Martin Sjölund]

The failing models in the testsuite with those changes:

./simulation/libraries/msl32/Modelica.Blocks.Examples.Filter.mos
./simulation/libraries/msl32/Modelica.Blocks.Examples.FilterWithRiseTime.mos
./simulation/libraries/msl32/Modelica.Electrical.Machines.Examples.SynchronousInductionMachines.SMEE_Rectifier.mos

It seems to also be the only models where this pattern applies.

[Lennart Ochel]

The model from the description works fine as well as the ones Martin listed. Is this just about generating nicer equations in the flat code and maybe simplify back end work or is there a actually defect?

[Martin Sjölund]

Yes, it's about making something simpler for the back-end (since it works for us, I guess simpler for WSM).

With my changes, the flattened code is simpler, but we generate bad code:

Modelica.Blocks.Examples.Filter_06inz.c:1154:31: error: assigning to
      'modelica_real' (aka 'double') from incompatible type 'real_array_t' (aka
      'struct base_array_s')
  ...= div_alloc_real_array(sub_alloc_real_array(omc_Modelica_Blocks_Continuous_Internal_Filter_roots_lowPass(threadData, tmp31, _OMC_LIT18, _OMC_LIT19, $PCriticalDamping$Pf_cut, NULL, NULL, NULL), tmp32), _OMC_LIT20);

So there is a bug because if we make things simpler, the back-end does unexpected things.

I will make the commit that changes from empty arrays to DAE.CREF(DAE.WILD()) since our back-end handles that. But I suggest we keep this ticket open until our back-end handles changing the call to simply be:

  CriticalDamping.r = Modelica.Blocks.Continuous.Internal.Filter.roots.lowPass({CriticalDamping.cr[1], CriticalDamping.cr[2], CriticalDamping.cr[3]}, {}, {}, CriticalDamping.f_cut);

Instead of:

  (CriticalDamping.r, _, _, _) = Modelica.Blocks.Continuous.Internal.Filter.roots.lowPass({CriticalDamping.cr[1], CriticalDamping.cr[2], CriticalDamping.cr[3]}, {}, {}, CriticalDamping.f_cut);

[Martin Sjölund]

r22700 has the changes suggested for the front-end. Still not optimizing everything away.

[Martin Sjölund]

This ticket was not closed for 1.9.1, which has now been released. It was batch modified for milestone 1.9.2 (but maybe an empty milestone was more appropriate; feel free to change it).

[Martin Sjölund]

Milestone changed to 1.9.3 since 1.9.2 was released.

[Martin Sjölund]

Moved to new milestone 1.9.4

[Martin Sjölund]

Milestone pushed to 1.9.5

[Martin Sjölund]

Milestone renamed

[Martin Sjölund]

Ticket retargeted after milestone closed

[Martin Sjölund]

Milestone moved to 1.12.0 due to 1.11.0 already being released.

[Francesco Casella]

The milestone of this ticket has been reassigned to "Future".

If you think the issue is still valid and relevant for you, please select milestone 1.13.0 for back-end, code generation and run-time issues, or 2.0.0 for front-end issues.

If you are aware that the problem is no longer present, please select the milestone corresponding to the version of OMC you used to check that, and set the status to "worksforme".

In both cases, a short informative comment would be welcome.

[Francesco Casella]

Given comment:12, I guess we can close this. Feel free to reopen if necessary.