Opened 11 years ago

Closed 11 years ago

#2429 closed defect (fixed)

Erroneous sign of a flow variable

Reported by: massimo ceraolo Owned by: Willi Braun
Priority: blocker Milestone: 1.9.1
Component: Backend Version: trunk
Keywords: Cc: Willi Braun, Lennart Ochel, Jens Frenkel

Description (last modified by massimo ceraolo)

In the attached model "asmaFlow" it should be:
aimc.flange.tau=aimc.inertiaRotor.flange_b.tau

instead it is:
aimc.flange.tau=-aimc.inertiaRotor.flange_b.tau

I think this is Critical since it causes a formally valid simulator to be created, but that gives erroneous results.

Attachments (5)

asmaFlow.mo (4.3 KB ) - added by massimo ceraolo 11 years ago.
asmaFlowTotalSCode.mo (48.3 KB ) - added by Adrian Pop 11 years ago.
Total model of the asmaFlow
trace.txt (305.3 KB ) - added by Adrian Pop 11 years ago.
instantiation of asmaFlow + dumpindxdae
asmaFLowDym.mat (436.4 KB ) - added by massimo ceraolo 11 years ago.
RemoveSimpleEquations.mo.patch (1.9 KB ) - added by Jens Frenkel 11 years ago.
patch for #2429

Download all attachments as: .zip

Change History (25)

by massimo ceraolo, 11 years ago

Attachment: asmaFlow.mo added

comment:1 by massimo ceraolo, 11 years ago

Description: modified (diff)
Priority: highcritical

comment:2 by Adrian Pop, 11 years ago

Hm, how is that? I get this where there is no equation like you say:

(-aimc.flange.tau) + aimc.inertiaRotor.flange_b.tau + aimc.friction.flange.tau + aimc.strayLoad.flange.tau = 0.0;

comment:3 by Adrian Pop, 11 years ago

Component: BackendFrontend
Owner: changed from probably noone to somebody

comment:4 by Adrian Pop, 11 years ago

Cc: Per Östlund added

Anyhow you have this equation in PartialBasicMachine:

connect(inertiaRotor.flange_b, flange);

where (if I can still interpret the Modelica Spec rules for connections right):
flange is an outside connector (- sign) and inertiaRotor.flange_b is an inside connector + sign so this will give you a set:
{<aimc.flange.tau, outside>, <aimc.inertialRotor.flange_b.tau, inside>}
So the equation above
(-aimc.flange.tau) + aimc.inertiaRotor.flange_b.tau ... = 0
is correct (as is combined with the other connection set which has only inside connectors with + sign).

I also cc Per as he's our expert on connections. Per, any comments on this?

by Adrian Pop, 11 years ago

Attachment: asmaFlowTotalSCode.mo added

Total model of the asmaFlow

by Adrian Pop, 11 years ago

Attachment: trace.txt added

instantiation of asmaFlow + dumpindxdae

comment:5 by massimo ceraolo, 11 years ago

What is strange is that, despite of the correct equation you mention, ardpo:

(-aimc.flange.tau) + aimc.inertiaRotor.flange_b.tau ... = 0

in the plots I see numbers that indicate that the following equation is actually true:

aimc.flange.tau + aimc.inertiaRotor.flange_b.tau = 0

that is wrong (the other terms summed in the equation are zero or negligible).

The issue can be checked also simulating the model with Dymola, that gives the correct result.
I will try to see if I'm able to produce a simpler example showing the issue (not sure).

I consider this issue serious since OM does simulate the model, but gives wrong result: this behavior tends to undermine the user's trust of OM.

comment:6 by massimo ceraolo, 11 years ago

I have an additional result.
If we remove from the model the speed sensor "speedSensor" the result becomes correct!
(therefore the quantity inertiaRotor.flange_b.tau changes from +15Nm to -15Nm).
So the presence of a sensor totally alters the result.

comment:7 by Adrian Pop, 11 years ago

Cc: Willi Braun Lennart Ochel added; Per Östlund removed
Component: FrontendBackend
Owner: changed from somebody to Willi Braun
Status: newassigned

Ok. The front-end seems to generate the correct equations.
Maybe the issue is in the simplification or the back-end.
If you look into trace.txt attached here (the dumpindxdae part)
we can see that aimc.flange.tau is an alias for -(-const.k)

251: aimc.flange.tau:VARIABLE(flow=true )  = -(-const.k)

and aimc.inertiaRotor.flange_b.tau is an alias for -const.k

267: aimc.inertiaRotor.flange_b.tau:VARIABLE(flow=true )  = -const.k

Do you know which of these is wrong?
I guess the back-end does something wrong here, but I don't quite get what it is.

I'll move this from front-end to the back-end

comment:8 by Adrian Pop, 11 years ago

As far as I can tell the plot should display:

  • -const.k for aimc.inertiaRotor.flange_b.tau
  • const.k for aimc.flange.tau

Maybe this is a problem on how we plot (or calculate) alias variables from the .mat file.
I mean the simulation goes fine but the plot (or read/write from/to the .mat file) is wrong.

Massimo, could you open and plot the .mat file generated by OpenModelica with Dymola and see if the results in the file are actually correct or not?

comment:9 by Adrian Pop, 11 years ago

Massimo, what stopTime do you give when you simulate the model?

comment:10 by Adrian Pop, 11 years ago

Nevermind, you have this in the model:
experiment(StopTime = 10, NumberOfIntervals = 10000)

comment:11 by Adrian Pop, 11 years ago

In the .mat file the results are the same for both:
aimc.inertiaRotor.flange_b.tau and aimc.flange.tau respectively -15

I think this is a bug in alias removal.
I think RemoveSimpleEquations somehow handles -(-e) a bit wrong (as negated alias).

Willi, can you have a look?

Last edited 11 years ago by Adrian Pop (previous) (diff)

comment:12 by Adrian Pop, 11 years ago

I think I found the issue. I'll let you know as soon as I test a bit.

comment:13 by massimo ceraolo, 11 years ago

Sorry for not answering earlier.
I've never looked at OM _res.mat files with Dymola before today, and indeed it seems that something is bad (maybe I make some mistake).

Indeed the plot file that OM creates from asmaFlow seems to be faulty. In OMEdit, for instance, if I try to see
aims.inertiaRotor.flange_a.tau
I don't see anything.

Indeed the file I proposed yesterday, when I opened this ticket, came from a simplification of another one, in which I could see all variables.
That file was faulty also.
When this ticket is solved, I will check what happens with the original file, to see if that one is solved also or something wrong remains.

*
Just in case it can help, I add the mat file that Dymola creates from asmaFlow.mo (for a simulation lasting 1s and having 1000 output points). It is named asmaFlowDym.mat.

Massimo

Last edited 11 years ago by massimo ceraolo (previous) (diff)

by massimo ceraolo, 11 years ago

Attachment: asmaFLowDym.mat added

comment:14 by Adrian Pop, 11 years ago

I'm working on a fix for this issue but is more complicated that I though.
I quite know what the problem is now but it will take a while to implement a fix.
I'll continue on it tomorrow and let you know when is fixed.

comment:15 by Adrian Pop, 11 years ago

Priority: criticalblocker
model MinimalModel
 Real a;
 Real b = 0.0;
 Real c;
 Real d;
 parameter Real const_k(start = 1.0) = -15.0;
equation
 c = -d;
 d + (-const_k) = b;
 c + a = b;
end MinimalModel;

comment:16 by Adrian Pop, 11 years ago

I made a minimal model that has this bug.
I now assigned this bug to Willi.

comment:17 by Adrian Pop, 11 years ago

Cc: Jens Frenkel added

Added Jens also to CC maybe he has some ideas on what might be wrong.
Me and Willi looked at RemoveSimpleEquations.mo for quite some time and we don't get what is wrong (I think there is something wrong with the negation, Willi thinks is something wrong with the order in which we apply the replacements).

comment:18 by Jens Frenkel, 11 years ago

  • fixed the bug local but svn take some time to load it up.

by Jens Frenkel, 11 years ago

patch for #2429

comment:19 by Adrian Pop, 11 years ago

Thanks Jens, yes, SVN is a bit slow because Hudson (we start Hudson jobs from SVN post commit hooks) has some issues that I try to fix at the moment. I'll run the testsuite with your patch and see how it goes.

comment:20 by Adrian Pop, 11 years ago

Resolution: fixed
Status: assignedclosed

Fixed in r17836 by Jens.

Note: See TracTickets for help on using tickets.