Opened 5 years ago
Last modified 3 years ago
#6003 assigned defect
Code generation bug
Reported by: | Owned by: | Karim Adbdelhak | |
---|---|---|---|
Priority: | high | Milestone: | |
Component: | Backend | Version: | v1.14.1 |
Keywords: | Cc: |
Description
Hi,
For a project at my new job I decided to move to openmodelica. My first experiment is an ideal time-division multiplexing system. It passes multiple signals through a single signal path during consecutive intervals.
To make the model more realistic, I added a low-pass filter in the signal path. Now I get a compilation error:
test_error_02nls.c: In function 'initializeStaticDataNLS14':
test_error_02nls.c:45:87: error: 'INTEGER_ATTRIBUTE {aka struct INTEGER_ATTRIBUTE}' has no member named 'nominal'
sysData->nominal[i] = data->modelData->integerVarsData[0].attribute /* MpxIndex.y */.nominal;
Previous reports of this error message seem to be related to an algebraic loop. Lo and behold: inserting a unit delay before the filter did solve the problem.
I find this strange, since at the model level it does not contain any loops.
The most surprising thing is that in any other simulation program, a low-pass filter breaks an algebraic loop because it places an integrator in the loop. Here it creates an algebraic loop!
I loaded the model in a trial version of dymola. Dymola has no problem with the model. Therefore I consider this an openmodelica bug.
I'll enclose a demo project (main model: test_error.mo)
Best regards,
Paul van der Hulst
PS I'm a newbie regarding (open)modelica, but have many years of experience simulating switched systems with simulink and various other simulation packages, including modification of an RK4 integration engine to properly handle state-events.
Attachments (1)
Change History (11)
by , 5 years ago
Attachment: | Modelica_test.zip added |
---|
comment:1 by , 5 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
Running the code through the transformational debugger says this variable is defined in the initial system as:
(residual) CryoMultiplexer.y - CryoMultiplexer.u[MpxIndex.y] = 0
That is, it's trying to figure out its value by looking through some discrete indexes of the input?
Adding an initial equation for the output seems to work, but I'm not sure myself how and why these things should work.
The equations for CryoMultiplexer.y were:
inital:
(assign) CryoMultiplexer.y := LPfilter.uu[1] * LPfilter.u_nominal
continuous-time:
(assign) CryoMultiplexer.y := CryoMultiplexer.u[MpxIndex.y]
So somehow Modelica thinks that this equation is somehow a good initial value for MpxIndex.y :) Someone else can explain why.
comment:2 by , 5 years ago
The basic problem is that MpxIndex.y
has no initial value and needs to be determined during intialization. The algorithm is not smart enough to figure out by itself that it needs to be fixed (this is a heuristic so you need to help it a little bit).
Since you already provided a start value for this discrete value you seem to know what it should look like at the start. Just fix it with fixed=true
and everything works fine.
In general: each state and each discrete variable needs to have either a fixed start value or an initial equation (which does not necessarily have to contain it, e.g. steady state initialization der(x) = 0.0
. it just needs to be logically connected). I would not rely on the tool to figure out these things on its own, that can be weird sometimes.
The only thing i changed to get it running is updating line 5 and 6 in test_error.mo
to be
TDMclock MpxIndex(nr_channels = nr_channels, period = T_sample, y(fixed=true)) annotation ( Placement(visible = true, transformation(origin = {-68, 86}, extent = {{-10, -10}, {10, 10}}, rotation = 0)));
fixing MpxIndex.y
. This should be the most correct way to do it.
Also consider fixing this value inside TDMclock
itself. I do not see an application where you would use this unfixed.
follow-up: 4 comment:3 by , 5 years ago
Hi,
Thanks for the suggestions.
Since the model compiles now, I can continue. Priority can be lowered as far as I'm concerned.
However, I do not understand if you consider this a bug or not. Dymola ran without complaints. Although I immediately accept that my model isn't very good, I don't think that the model is in violation with the language.
Openmodelica itself also did not detect any errors.
To say the least: the (compiler) error message was less than helpful.
Best regards,
Paul
comment:4 by , 5 years ago
Replying to p.van.der.hulst@…:
Hi,
Thanks for the suggestions.
Since the model compiles now, I can continue. Priority can be lowered as far as I'm concerned.
However, I do not understand if you consider this a bug or not. Dymola ran without complaints. Although I immediately accept that my model isn't very good, I don't think that the model is in violation with the language.
Openmodelica itself also did not detect any errors.
To say the least: the (compiler) error message was less than helpful.
Best regards,
Paul
As i said, it is a heuristic. It can probably be improved by forcing discrete variables to be solved explicitly and if it is not possible they have to be fixed by the compiler. Right now it only prefers discrete variables to be solved explicitly but does not force the fixing if it is not possible. In this particular case it should prevent the problem.
We did not really focus on problems like these since it only matters in fringe cases, most of the time discrete variables have an explicit way to be solved. But it is an enhancement that could be made for sure. For starters, i could add a better error message which points to the discrete variable which the compiler tries to solve implicitly and propose setting fixed = true
, or providing a proper explicit initial equation.
follow-up: 7 comment:5 by , 5 years ago
@Karim, do we have some warnings about missing initial equation for discrete variables? We have this for continuous states, but I am not sure about discrete ones.
If we don't, we should, and we should make clear that this is a potentially critical situation, that you should avoid by explicitly setting fixed = true
or adding initial equations.
comment:6 by , 5 years ago
Component: | Code Generation → Backend |
---|---|
Milestone: | → 1.17.0 |
comment:7 by , 5 years ago
Replying to casella:
@Karim, do we have some warnings about missing initial equation for discrete variables? We have this for continuous states, but I am not sure about discrete ones.
If we don't, we should, and we should make clear that this is a potentially critical situation, that you should avoid by explicitly setting
fixed = true
or adding initial equations.
No i don't think that we have something like that. Probably easy to add though. But since you scheduled that for v1.17.0 i will not focus on it now.
comment:9 by , 4 years ago
Milestone: | 1.17.0 → 1.18.0 |
---|
Retargeted to 1.18.0 because of 1.17.0 timed release.
Demonstration of openmodelica generating erroneous code