Merge modifiers from the original component when redeclaring components.

[Christian Schubert]

I am testing the new instantiation with MSL 3.2.1 and noted a strange behaviour with Modelica.Electrical.Analog.Examples.CompareTransformers.

It simulates with both the new and the old instantiation. However, when running the test Modelica.Electrical.Analog.Examples.CompareTransformers.mos, the omc goes into an infinte loop when using the new instantiation. Having looked a little bit more into it, I found that the omc hangs when comparing the results?!

Maybe someone could have a look at it.

[Jens Frenkel]

Because there are only 3 Variables could you please try to remove one ore two and see if the error is still there?

basicTransformer.i1
basicTransformer.i2
inductor21.i

And generate with +d=dumpindxdae a trace for old version and new version.

[Christian Schubert]

OK, I now looked a bit more into it. The results are fundamentally different and due to small timesteps, the resultfile is quite big and the omc seems to hang when writing the diff file.
When running omc with +d=dumpdaelow I get the following differences:
Old Instantiation:

37: sineVoltage2.signalSource.startTime:PARAM()  = sineVoltage2.startTime .Modelica.Electrical.Analog.Examples.CompareTransformers, .Modelica.Electrical.Analog.Sources.SineVoltage, .Modelica.Blocks.Sources.Sine, .Modelica.SIunits.Time type: Real ()
38: sineVoltage2.signalSource.offset:PARAM()  = sineVoltage2.offset .Modelica.Electrical.Analog.Examples.CompareTransformers, .Modelica.Electrical.Analog.Sources.SineVoltage, .Modelica.Blocks.Sources.Sine, .Real type: Real ()
48: sineVoltage1.signalSource.startTime:PARAM()  = sineVoltage1.startTime .Modelica.Electrical.Analog.Examples.CompareTransformers, .Modelica.Electrical.Analog.Sources.SineVoltage, .Modelica.Blocks.Sources.Sine, .Modelica.SIunits.Time type: Real ()
49: sineVoltage1.signalSource.offset:PARAM()  = sineVoltage1.offset .Modelica.Electrical.Analog.Examples.CompareTransformers, .Modelica.Electrical.Analog.Sources.SineVoltage, .Modelica.Blocks.Sources.Sine, .Real type: Real ()

whereas with the new instantiation I get

37:  sineVoltage2.signalSource.startTime:PARAM()  = 0.0 .Modelica.Electrical.Analog.Examples.CompareTransformers, .Modelica.Electrical.Analog.Sources.SineVoltage__OMC__1, .Modelica.Blocks.Sources.Sine, .Modelica.SIunits.Time type: Real ()
38: sineVoltage2.signalSource.offset:PARAM()  = 0.0 .Modelica.Electrical.Analog.Examples.CompareTransformers, .Modelica.Electrical.Analog.Sources.SineVoltage__OMC__1, .Modelica.Blocks.Sources.Sine, .Real type: Real ()
48: sineVoltage1.signalSource.startTime:PARAM()  = 0.0 .Modelica.Electrical.Analog.Examples.CompareTransformers, .Modelica.Electrical.Analog.Sources.SineVoltage__OMC__3, .Modelica.Blocks.Sources.Sine, .Modelica.SIunits.Time type: Real ()
49: sineVoltage1.signalSource.offset:PARAM()  = 0.0 .Modelica.Electrical.Analog.Examples.CompareTransformers, .Modelica.Electrical.Analog.Sources.SineVoltage__OMC__3, .Modelica.Blocks.Sources.Sine, .Real type: Real ()

although sineVoltage1.offset = sineVoltage2.offset = Vdc = 0.1

[Adrian Pop]

Hi,

Ok. It seems the SCode->SCode translation does something wrong in this case.
I'll have a look if I can fix this.

[Christian Schubert]

I attached a test for the problem.
It is got to do with final parameters in a replaceable statement

replaceable C c(final offset = offset);

When you redeclare c, the binding equation offset = offset should be added implicitly to redeclaration.
Without scodeInstShortcut we get

parameter Real c.offset = offset;

and with scodeInstShortcut enabled we get

parameter Real c.offset = 0.0;

[Adrian Pop]

No, we don't want Martin here.

[Adrian Pop]

I'm already working on debugging this so hopefully I'll fix it soon.

[Adrian Pop]

It seems this also happens with the new inst:

adrpo@ida-liu050 /c/dev/avm
$ time ~/dev/OpenModelicaNoChanges/build/bin/omc +locale=C +d=scodeInst ParameterBug.mos 
true
""
"class A
  parameter Real Vdc = 1.0;
  parameter Real offset = Vdc;
  parameter Real c.offset = 0.0;
  Real c.x;
  Real c.y = c.x;
equation
  c.x = c.offset;
end A;

It seems we don't keep the existing modifier when we redeclare components.

[Adrian Pop]

I guess the when applying component redeclares to the env we should merge the modifiers from the redeclare with the ones from the original component (outer->inner).

[Adrian Pop]

*FlattenRedeclare.replaceElementInScope should do:

new_item = propagateItemPrefixes(old_item, inElement);
// here we should also do: merge(newElementModifiers, olElementModifiers) -> newModifs
// apply the newModifs to the newElement.

[Adrian Pop]

Hmm, i wonder what the specification say about this.
Should we keep the original modifiers when we redeclare the components?

extends B(redeclare D c);

should remove the modifiers from:

replaceable C c(final offset = offset);

or it should keep them?
As far as I know the original prefixes are kept,
so are the array dimensions, so maybe this is
the case for modifiers too.

[Per Östlund]

Replying to adrpo:

Hmm, i wonder what the specification say about this.

The specification doesn't explicitly say anything about what happens with the modifiers it seems, but the examples implies that modifiers should be kept. Whether they should be kept only from the original declaration or if they are merged from all redeclares in the case of multi-level redeclares is anyones guess though.

[Per Östlund]

Or actually, perhaps it helps to view redeclares and modifications separately? I.e. extends A(redeclare B b = 5) is the same as extends A(redeclare B b, b = 5). Perhaps splitting modifiers in that way would fix the issue?

[Adrian Pop]

Per fixed some of these issues in #1989.
I'll look into how to apply these changes to +d=scodeInstShortcut.

[Adrian Pop]

Fixed in #14484.