More issues with Complex multiplication in NF

[casella]

Please run the following model with the NF

model TestSqrt
  Complex z,w;
equation
  z = Complex(time, 1);
  w = sqrt(3)*z;
end TestSqrt;

The following error is reported:

TestSqrt_functions.c: In function 'omc_omplex__omcQuot_2A_multiply__eval1':
TestSqrt_functions.c:56:3: error: '_c3' undeclared (first use in this function)
   _c3._re = tmp1._re;
   ^
TestSqrt_functions.c:56:3: note: each undeclared identifier is reported only once for each function it appears in

With the old FE, the model is flattened to

function Complex.'*'.multiply "Inline before index reduction" "Multiply two complex numbers"
  input Complex c1 "Complex number 1";
  input Complex c2 "Complex number 2";
  output Complex c3 "= c1*c2";
algorithm
  c3 := Complex(c1.re * c2.re - c1.im * c2.im, c1.re * c2.im + c1.im * c2.re);
end Complex.'*'.multiply;

class TestSqrt
  Real z.re "Real part of complex number";
  Real z.im "Imaginary part of complex number";
  Real w.re "Real part of complex number";
  Real w.im "Imaginary part of complex number";
equation
  z.re = time;
  z.im = 1.0;
  w = Complex.'*'.multiply(Complex.'constructor'.fromReal(1.732050807568877, 0.0), z);
end TestSqrt;

while the NF gives

function Complex.'*'.multiply "Inline before index reduction" "Multiply two complex numbers"
  input Complex c1 "Complex number 1";
  input Complex c2 "Complex number 2";
  output Complex c3 "= c1*c2";
algorithm
  c3 := Complex.'constructor'.fromReal(c1.re * c2.re - c1.im * c2.im, c1.re * c2.im + c1.im * c2.re);
end Complex.'*'.multiply;

class TestSqrt
  Real z.re "Real part of complex number";
  Real z.im "Imaginary part of complex number";
  Real w.re "Real part of complex number";
  Real w.im "Imaginary part of complex number";
equation
  z = Complex.'constructor'.fromReal(time, 1.0);
  w = Complex.'*'.multiply(Complex.'constructor'.fromReal(1.732050807568877, 0.0), z);
end TestSqrt;

First of all, the NF does not expand the first equation into its scalar components, which may later prevent futher symbolic manipulation (e.g. symbolic Jacobian computation).

Furthermore, it does not expand the Complex constructor in the body of the multiplication function algorithm, which probably causes some problem with the generated code.

BTW, it would be really good if also the Complex equation involving w was eventually expanded into its scalar components, even if the old FE didn't do that. This would again allow for more optimizations in the code generation phase.

[perost]

This seems like a code generation and/or inlining issue. It generates the variables _c3_re and _c3_im in the C code, but uses _c3._re and _c3._im for the calcaluations. It also returns _c3_re instead of e.g. _c3. But the function should really be inlined by the backend, which doesn't happen for some reason.

The reason this doesn't happen with the old frontend is because it erroneously uses the default constructor for Complex, which is much easier to evaluate or inline. This happens to work for Complex since the overloaded constructor is the same as the default one, just with a default argument, but will give an incorrect result if the overloaded constructor doesn't behave exactly as the default one.

Also, the backend is currently responsible for inlining functions marked Inline = true, the frontends only inline functions marked __OpenModelica_EarlyInline = true. I'm assuming this is done early enough for the relevant optimizations to be applied, so I don't think there's any need for the frontend to do any more inlining than it currently does.

[perost]

Actually this seems to be an NF issue after all, the '*' call isn't marked as inline like it should.

[casella]

Replying to perost:

Actually this seems to be an NF issue after all, the '*' call isn't marked as inline like it should.

Aha :)

Is it that simple a fix then?

[perost]

Replying to casella:

Replying to perost:

Actually this seems to be an NF issue after all, the '*' call isn't marked as inline like it should.

Aha :)

Is it that simple a fix then?

Yes, very simple. It's just that the operator overloading code manually creates calls and doesn't bother filling in correct call attributes. I just need to change it to use the proper function for creating calls instead. I'll push in a fix for it in a moment, I'm just making really sure we don't have any other such issues elsewhere.

[perost]

Fixed in 2dc3bec6 (when Hudson gets around to it), the backend now inlines the calls properly with the NF and the model compiles and simulates.

The code generation issue is still not fixed though, but I can't replicate it now. Oddly enough it works fine even if I remove all the inline annotations, it seems like the backend inlines the calls anyway in that case. So I'll close this ticket since the main issue is fixed, and if someone manages to replicate the code generation issue they can open a new ticket.