Complex 'sum' does not work

[Christian Kral <dr.christian.kral@…>]

Please consider the following example:

model TestComplexSum
  parameter Complex a[2]={Complex(1,2),Complex(2,3)};
  parameter Complex sum1 = Modelica.ComplexMath.'sum'(a);
  parameter Complex sum2 = Complex(sum(a.re),sum(a.im));
end TestComplexSum;

The sum of vector a is calculated in two different ways, so the results shall be equal. However, in OpenModelica sum2 is calculated correctly, whereas sum1 results in

sum1.re -> 0
sum2.re -> 0

Too bad: The application of function 'sum' is utilized in the actual trunk version of the MSL in order to calculate the the total apparent power in Modelica.Electrical.QuasiStationary.MultiPhase.Sensors.MultiSensor. I would not really like to switch to the alternative calculation of sum1...

Do you think there is a chance, that this issue is going to be fixed in OpenModelica 1.12? The reason I am asking is that I am planning to refer to OpenModelica in a publication so, so I am not sure if can include a related example or not.

[Martin Sjölund]

Modifying ComplexMath.'sum' to be the same as sum2 instead causes the record equation to be lost... The problem is in the frontend.

[Martin Sjölund]

I had a look at this today, and part of the problem is that the binding can't be split due to it being a record. And if we move the binding to the initial equation section, there is a problem because the parameter is not fixed. Which means we would need to change that or introduce some new concept for handling bindings of initial record equations in a special way.

There is also a problem with v[:].re, which seems to return the empty array {} (which means a constant 0 as the sum).

And there is additionally a problem with the inline operation: it seems this becomes sum(v.re) rather than sum(a.re).

I would guess we can't fix all of these issues in time for 1.12 (especially considering we were supposed to feature freeze that and move it to the maintenance branch some month(s) ago).

[Martin Sjölund]

There is also a problem with code generation for the expression v.re, and it looks like the problem there is that the code generator thinks v.re is a scalar. It's also not possible to rewrite the expression as sum(c.re for c in v).

[massimo ceraolo]

I would guess we can't fix all of these issues in time for 1.12 (especially considering we were supposed to feature freeze that and move it to the maintenance branch some month(s) ago).

There are sevetal tickets open on complex numbers.
Maybe a plan can be made after 1.12 release to address most of them?
It seems that better complex nmber support is wished by many (I'm one of them)

[Christian Kral <dr.christian.kral@…>]

Thanks for the feedback.

An improved complex number support is quite important to me and the classes I am teaching. I use OpenModelica a lot on quasi static analysis.

[Francesco Casella]

It would be good to be sure that at least the new front-end deals with this correctly.

Maybe Mahder could have a look?

[Christian Kral <dr.christian.kral@…>]

Created alternative implementation, see:
​https://github.com/modelica/Modelica/issues/2201
​https://github.com/modelica/Modelica/commit/c4ba91f078703d7b5a4daeefd48d4c23389bf89e

[Christian Kral <dr.christian.kral@…>]

This is a workaround to overcome issues of Modelica.Electrical.QuasiStationary.MultiPhase.Sensors.MultiSensor

[Martin Sjölund]

The following would work in ​v1.13 (not 1.12), but would give additional messages:

function 'sum' "Return sum of complex vector"
  input Complex v[:] "Vector";
  output Complex result "Complex sum of vector elements";
algorithm
  result:=Complex(sum(v[i].re for i in 1:size(v,1)), sum(v[i].im for i in 1:size(v,1)));
  annotation(Inline=true);
end 'sum';

model TestComplexSum1
  parameter Complex a[2]={Complex(1,2),Complex(2,3)};
  parameter Complex sum1 = 'sum'(a);
  parameter Complex sum2 = Complex(sum(a.re),sum(a.im));
end TestComplexSum1;

Notification: Moving binding to equation section and setting fixed attribute of sum1.re to false. This is done to work around a compiler design flaw (there is no good way to represent bindings that span multiple variables).
Notification: Moving binding to equation section and setting fixed attribute of sum1.im to false. This is done to work around a compiler design flaw (there is no good way to represent bindings that span multiple variables).

[Francesco Casella]

Wow. I think we should remove the phrase:

This is done to work around a compiler design flaw (there is no good way to
represent bindings that span multiple variables).

as it may scare off the user unnecessarily. Also, there's no need to advertise compiler design flaws if there are good workarounds; moving a parameter binding equation to the initial equation section is ok.

Last but not least, I guess you should write "Moving binding to *initial* equation section...".

[Martin Sjölund]

Fine... ​https://github.com/OpenModelica/OMCompiler/pull/1912

[Christian Kral <dr.christian.kral@…>]

An additional issue occurred in model ​https://github.com/christiankral/HanserModelica/blob/master/HanserModelica/SynchronousMachines/SMPM_MTPA.mo

I therefore changed the implementation of the quasi static ToSpacePhasor block: ​https://github.com/modelica/Modelica/commit/98cc538418c4a2466c6fbbb095e265917ba55b75

[dr.christian.kral@…]

Due to a wrong calculation of the complex sum in the quasi static current sensor (was calculated to be zero in OM, even though it shall be non-zero) in model ​https://github.com/christiankral/HanserModelica/blob/master/HanserModelica/InductionMachines/IMC_DOL.mo I changed the implementation of the calculation of the complex sum in:

• ​https://github.com/modelica/Modelica/commit/f7dc14dc5dafb6cf84085666396ffebe519fd6b4
• ​https://github.com/modelica/Modelica/commit/7369c0fe6ea6da8c3240d907b679141ea66cce67

This is a workaround for now, even though the next release of OM may not be affected by changes of the MSL trunk or maint/3.2.2.

Does anyone have a suggestion how this issue can be targeted in a better way?

[Francesco Casella]

I gave a fresh look at this issue with the new front end. If you run the TestComplexSum model with -d=newInst, the result is a compile-time error:

TestComplexSum_functions.c: In function 'omc_Modelica_ComplexMath__omcQuot_73756D':
TestComplexSum_functions.c:54:154: error: expected expression before ')' token
   tmp3 = omc_Complex__omcQuot_636F6E7374727563746F72_fromReal(threadData, sum_real_array(((Complex*)(generic_array_element_addr(&_v, sizeof(Complex), 1, )))->_re), sum_real_array(((Complex*)(generic_array_element_addr(&_v, sizeof(Complex), 1, )))->_im));

The flattened model contains calls to functions that are meant to be inlined:

class TestComplexSum
  parameter Real a[1].re(fixed = false) "Real part of complex number";
  parameter Real a[1].im(fixed = false) "Imaginary part of complex number";
  parameter Real a[2].re(fixed = false) "Real part of complex number";
  parameter Real a[2].im(fixed = false) "Imaginary part of complex number";
  parameter Real sum1.re(fixed = false) "Real part of complex number";
  parameter Real sum1.im(fixed = false) "Imaginary part of complex number";
  parameter Real sum2.re(fixed = false) "Real part of complex number";
  parameter Real sum2.im(fixed = false) "Imaginary part of complex number";
initial equation
  sum2 = Complex.'constructor'.fromReal(sum(a.re), sum(a.im));
  sum1 = Modelica.ComplexMath.'sum'(a);
  a[2] = Complex.'constructor'.fromReal(2.0, 3.0);
  a[1] = Complex.'constructor'.fromReal(1.0, 2.0);
end TestComplexSum;

Unfortunately, the inlining does not take place, as it is apparent from the output of -d=optdaedump

Equations (4, 8) 
======================================== 
1/1 (2): sum2 = Complex(sum(a.re), sum(a.im))   [dynamic |0|0|0|0|] 
2/3 (2): sum1 = Modelica.ComplexMath.'sum'(a)   [dynamic |0|0|0|0|] 
3/5 (2): a[2] = Complex(2.0, 3.0)   [dynamic |0|0|0|0|] 
4/7 (2): a[1] = Complex(1.0, 2.0)   [dynamic |0|0|0|0|] 

I understand in this case the backend should inline the function calls, eventually reducing the initial equations for the parameter values to eight scalar equations. Maybe the inline annotation gets lost, @perost can you please check?

[Francesco Casella]

One of the reasons of this failure could be #4354: initial equations are not inlined, and this causes issues.

[Francesco Casella]

See also #5236

[Francesco Casella]

Running this model with the latest nightly an the NF I now obtain this NF error:

[1] 23:52:32 Translation Error
[C:/dev/OM64bit/OMCompiler/Compiler/NFFrontEnd/NFEvalFunction.mo: 387:9-388:53]:
Internal error NFEvalFunction.applyReplacementCref could not find
replacement for v[:].re

[Francesco Casella]

I now get the following error during C compilation

TestComplexSum_functions.c: In function 'omc_Modelica_ComplexMath__omcQuot_2773756D27':
TestComplexSum_functions.c:55:158: error: expected expression before ')' token
   tmp3 = omc_Complex__omcQuot_27636F6E7374727563746F7227_fromReal(threadData, sum_real_array(((Complex*)(generic_array_element_addr(&_v, sizeof(Complex), 1, )))->_re), sum_real_array(((Complex*)(generic_array_element_addr(&_v, sizeof(Complex), 1, )))->_im));
                                                                                                                                                              ^

[Francesco Casella]

The flattened model is:

class TestComplexSum
  parameter Real a[1].re = 1.0 "Real part of complex number";
  parameter Real a[1].im = 2.0 "Imaginary part of complex number";
  parameter Real a[2].re = 2.0 "Real part of complex number";
  parameter Real a[2].im = 3.0 "Imaginary part of complex number";
  parameter Real sum1.re(fixed = false) "Real part of complex number";
  parameter Real sum1.im(fixed = false) "Imaginary part of complex number";
  parameter Real sum2.re(fixed = false) "Real part of complex number";
  parameter Real sum2.im(fixed = false) "Imaginary part of complex number";
initial equation
  sum1 = Modelica.ComplexMath.'sum'(a);
  sum2 = Complex.'constructor'.fromReal(a[1].re + a[2].re, a[1].im + a[2].im);
end TestComplexSum;

I guess with the current BE we need the NF to expand the two initial equations, particularly the first one. In the future, the array-preserving BE should be capable to handle these equations right away.