Opened 8 years ago

Closed 7 years ago

#3993 closed defect (fixed)

Back-end fails with MSL 3.2.2 TimeTable but works with MSL 3.2.1

Reported by: Adam Dershowitz Owned by: Volker Waurich
Priority: high Milestone: Future
Component: Backend Version:
Keywords: Cc: Patrick Täuber, Volker Waurich

Description

I have a model that worked fine until some recent upgrades. I'm not sure of the exact change, but it worked fine with with 1.9.4-dev-699 but fails with 1.10.0 and 1.11.0 versions.
The error that I get is:

[1] 15:48:09 Symbolic Warning
[Modelica.Blocks.Tables: 687:7-688:66]: Unused input variable tableAvailable in function .Modelica.Blocks.Tables.CombiTable2D$pipeHt1$frictionfactorTable2d.getDerTableValue.

[2] 15:48:09 Symbolic Warning
[Modelica.Blocks.Tables: 660:7-661:66]: Unused input variable tableAvailable in function .Modelica.Blocks.Tables.CombiTable2D$pipeHt1$frictionfactorTable2d.getTableValue.

[3] 15:48:09 Translation Error
Internal error It is not possible to select continues time states because Number of Equations 18 greater than number of States 13 to select from.

[4] 15:48:09 Translation Error
Internal error - IndexReduction.dynamicStateSelectionWork failed!

I believe that this is actually due to the upgrade of MSL to 3.2.2. But, I'm really not sure what this error means, or why a model that formerly worked should now report an issue with the number of equations. I was told that CombiTable2D was rewritten with the upgrade to 3.2.2 so perhaps that bug is there? Or perhaps there was a hidden bug somewhere else, and this change is just showing it?
Unfortunately, I can't post my code here, as it is proprietary.

Attachments (1)

foo.mo (261 bytes ) - added by Francesco Casella 7 years ago.

Download all attachments as: .zip

Change History (19)

comment:1 by anonymous, 8 years ago

relate possible to #2751

comment:2 by Adam Dershowitz, 8 years ago

I did just try to switch to MSL 3.2.1 and that does allow the model to run successfully. So, it is a bug in MSL 3.2.2.

comment:3 by dma@…, 7 years ago

I'm having the same problem! Is there a workaround or other solution to this? I don't think going back to 3.2.1 is practical for me.

in reply to:  2 comment:4 by Francesco Casella, 7 years ago

Replying to dersh:

I did just try to switch to MSL 3.2.1 and that does allow the model to run successfully. So, it is a bug in MSL 3.2.2.

I guess it's rather a bug in how OMC supports the code of the tables in there.

Replying to dma@…:

I'm having the same problem! Is there a workaround or other solution to this? I don't think going back to 3.2.1 is practical for me.

It would be really helpful if you could post a small test case here that reproduces the problem. Otherwise it's hard to pinpoint the problem and solve it effectively.

comment:5 by Adam Dershowitz, 7 years ago

@casella, I reached out via email, as I can't post my code here. I did verify that this problem still occurs in the current versions of OMEdit.

When I first ran into this problem, it was suggested that it might be related to this bug:
https://trac.openmodelica.org/OpenModelica/ticket/2751

comment:6 by Francesco Casella, 7 years ago

@dersh, I tried your model with the current nightly. The error I get with MSL 3.2.2 (not with MSL 3.2.1) is

[1] 14:56:47 Translation Error
Internal error It is not possible to select continues time states because Number of Equations 16 greater than number of States 11 to select from.

[2] 14:56:47 Translation Error
Internal error - IndexReduction.dynamicStateSelectionWork failed!

I understand your model needs some fancy index reduction, because the beaviour of the pipe model depends on the time derivative of one input.

This input is currently defined by a TimeTable source generator, and as far as I understand it was changes to that model that broke your model with MSL 3.2.2. If you replace the TimeTable generator by an equivalent Real expression (which the back-end can apparently handle more nicely), your model runs fine in MSL 3.2.2 as well.

I investigated the changes in TimeTable between MSL 3.2.1 and MSL 3.2.2: the timeScale factor was added, and there are conversions between time and scaled time inside an algorithm. Maybe that is causing some trouble to the back-end.

comment:7 by Francesco Casella, 7 years ago

I tried out a simple example involving index reduction (attached foo.mo) and it works, but I understand that @dersh's model involves dynamic state selection, which may be more involved. BTW, OMC's current support of dynamic state selection is still rather bumpy and incomplete.

by Francesco Casella, 7 years ago

Attachment: foo.mo added

comment:8 by Adam Dershowitz, 7 years ago

I tried to change the TimeTable to a ramp, and now the code does run under MSL 3.2.2, as you said. In both cases, I used a simple ramp, so I would have expected identical results. But, clearly a TimeTable, with only a start and finish value is generating different processing from a ramp.
As a user, is there any way to know that it is doing dynamic state selection, and how to address it?

Thanks for the assistance.

PS, I also fixed the unrelated variable issue, but that didn't seem to matter to OMC.

comment:9 by Francesco Casella, 7 years ago

Cc: Patrick Täuber Volker Waurich added; Willi Braun removed
Component: *unknown*Backend
Owner: changed from somebody to Lennart Ochel
Summary: Translation Error CombiTable2DBack-end fails with MSL 3.2.2 TimeTable but works with MSL 3.2.1

Changed the subject to reflect the root cause of the problem more precisely

comment:10 by Volker Waurich, 7 years ago

In msl321, the algorithm block which calls getInterpolationCoefficients contained discrete output variables only. In msl 322, the variable timeTable1.timeScaled has been added. This one is continuous. When deriving the algorithm block in IndexReduction, we get a new derivative equation inside the algorithm block which is problematic for the current backend implementation during state selection. I am trying to fix this, but its tedious. Variables and equations are counted wrong, Adajcency and incidence matrix generation fails...

comment:11 by Volker Waurich, 7 years ago

Ok. I hacked the compiler to translate and simulate the model but a lot of changes would be involved: Changing Differentiation of Algorithms, Changing Adjacency and IncidenceMatrix generation, change counting of algorithm sizes in some places. The core problem is to differentiate an algorithm block that involves states, discrete variables and continuous variables. How does the algorithm looks after differentiation? It works if the derived continuous parts of the algorithm are added as additional, separate equations and the discrete parts of the algorithm are dismissed.

comment:12 by Francesco Casella, 7 years ago

I wonder whether we should go this way, or rather change the implementation of TimeTable. For me, putting the time scaling in the algorithm is a very bad mistake, it should have been done outside in the equation section, where it is a trivial multiplication operation.

We can push this in MSL 3.2.3, which should be released in the near future. What do you think?

comment:13 by Volker Waurich, 7 years ago

I think it is useful to condition omc to handle derivations of algorithms properly during index reduction. Unfortunately, even if the assignment timeScaled = time/timeScale; is moved to the equation section, the when-clause is still part of the MSSS and will be derived since timeScaled is one of the arguments of getInterpolationCoefficients. I will try to adress the issue step by step.

comment:14 by Volker Waurich, 7 years ago

There is another way to get the model running with a slight model change.
Make timeScaled discrete by writing timeScaled = time/timeScale; inside the when-clause.

comment:15 by Francesco Casella, 7 years ago

@vwaurich, would you mind opening a pull request on the MSL with this modification?

comment:16 by Volker Waurich, 7 years ago

The problematic equation moved from the algorithm block to the equation block. It's already in the MSL master. This does not fix the issue but it makes it easier to adapt the compiler. I already pushed the omc fix. The model simulates now for the current MSL 3.2.2 master.

comment:17 by Volker Waurich, 7 years ago

Owner: changed from Lennart Ochel to Volker Waurich
Status: newassigned

comment:18 by Volker Waurich, 7 years ago

Resolution: fixed
Status: assignedclosed
Note: See TracTickets for help on using tickets.