Opened 6 years ago
Closed 6 years ago
#4971 closed defect (fixed)
More issues with Complex multiplication in NF
Reported by: | Francesco Casella | Owned by: | Per Östlund |
---|---|---|---|
Priority: | high | Milestone: | 2.0.0 |
Component: | New Instantiation | Version: | |
Keywords: | Cc: |
Description
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.
Change History (6)
comment:1 by , 6 years ago
Component: | New Instantiation → Backend |
---|---|
Owner: | changed from | to
comment:2 by , 6 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
follow-up: 4 comment:3 by , 6 years ago
Component: | Backend → New Instantiation |
---|
Actually this seems to be an NF issue after all, the '*'
call isn't marked as inline like it should.
follow-up: 5 comment:4 by , 6 years ago
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?
comment:5 by , 6 years ago
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.
comment:6 by , 6 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
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.
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.