Opened 9 years ago

Closed 7 years ago

#3446 closed defect (fixed)

Impure file name passing breaking CombiTimeTable

Reported by: paul.m.sco@… Owned by: lochel
Priority: normal Milestone: 1.12.0
Component: Backend Version: trunk
Keywords: impure Cc: lochel

Description

Failure in creating a CombiTimeTable from an ASCII file, where the file name for the table is produced by a function using the "impure" keyword. When the function is not labelled as being "impure" it works as expected.

When "impure" is used for the function returning the string, the simulation fails with the error saying that "usertab" is not implemented. Looking at the CombiTimeTable external C source code, it seems likely that somehow the C code is being passed "NoName", which it interprets as us trying to use "usertab". This might be a problem with calculating the value of the impure function at initialisation used in conjunction with external C code.

An example is attached along with a file that contains the ASCII matrix "file.ext".

Run it in shell:
loadModel(Modelica)
loadFile("impure.mo")
simulate(Impure)

To get it to work correctly remove the impure keyword from the first function. Note that I realise that the function in this example is not actually impure, but when the bug first cropped up I was using an impure function.

Attachments (2)

impure.mo (460 bytes) - added by anonymous 9 years ago.
file.ext (28 bytes) - added by anonymous 9 years ago.
ASCII matrix for example

Download all attachments as: .zip

Change History (22)

Changed 9 years ago by anonymous

Changed 9 years ago by anonymous

ASCII matrix for example

comment:1 Changed 9 years ago by sjoelund.se

  • Cc lochel added; v1.9.3-dev-723-g5992e4a-dirty removed
  • Component changed from Unknown to Backend

Some dependency issue for external objects. The implementation assumes that all constructor inputs are constants and known at compile-time (or parameters assigned when reading the input data). tab.fileName has value="" (the start-value) when evaluating the constructor. The constructor equations should also be part of the initialization in some way (sorted equations that do not form algebraic loops on external object constructors and that can simply be evaluated from top and down in order to initialize external objects exactly once).

comment:2 Changed 7 years ago by Christian Kral <dr.christian.kral@…>

I am running into a similar issue when running an example of the Buildings library. Please run Buildings.Electrical.AC.OnePhase.Sources.Examples.WindTurbine.

Compilation process finishes successfully but the simulation fails with:

stdout | error | <p>Simulation process failed. Exited with code 255.</p>
assert | debug | <p>Function "usertab" is not implemented<br>

Is this also an issue of impure or is this caused by the class CombiTable1Ds used in instance weaDat (which is somehow suggested by the message browser feedback to me)?

comment:3 Changed 7 years ago by Christian Kral <dr.christian.kral@…>

Possibly the missing usertab is related with #3363, as the keyword usertab also appears in the ticket comments of #3363.

comment:4 Changed 7 years ago by sjoelund.se

@lochel: Still got the same problem more than a year later.

Perhaps at least make a check that parameters used for the external object constructor are calculated and issue a warning/error before generating code. Better would be to put parameters not part of any algebraic loop before external objects are calculated so you can depend on non-constant parameters for external object constructors.

comment:5 Changed 7 years ago by lochel

@sjoelund.se: I just had a quick look. The issue is that the external object constructor is called before the actual parameter initialization. I have no experience with external objects yet, so is it possible that parameters depend on external objects? I guess not. In that case, we could just change the order of the parameter initialization and the external object constructor.

comment:6 follow-up: Changed 7 years ago by sjoelund.se

Yes, parameters are allowed to depend on external objects (this is the standard case). You must also call the constructor exactly once.

So you need to ensure that:

  • There is no algebraic loop for external objects and the parameters it depends on
  • Some parameters are initialized before external objects (let's call it pre-initialization, where you have a linear sequence of parameter and external object assignments)
  • After this, initilization including algebraic loops resumes

comment:7 Changed 7 years ago by sjoelund.se

Note that there has also been talk in the MA to allow algebraic loops for some external objects (and a re-initilize function).

comment:8 in reply to: ↑ 6 ; follow-up: Changed 7 years ago by lochel

Replying to sjoelund.se:

Yes, parameters are allowed to depend on external objects (this is the standard case). You must also call the constructor exactly once.

So you need to ensure that:

  • There is no algebraic loop for external objects and the parameters it depends on
  • Some parameters are initialized before external objects (let's call it pre-initialization, where you have a linear sequence of parameter and external object assignments)
  • After this, initilization including algebraic loops resumes

Well, that sounds like the exact same restriction as for primary parameters, i.e. those having fixed=true. Maybe we should just merge both routines, since we do this kind of checks already for these parameters. That would allow to have parameters depending on external object and vice versa, as long as there is no algebraic loop involved.

comment:9 in reply to: ↑ 8 Changed 7 years ago by sjoelund.se

Replying to lochel:

Well, that sounds like the exact same restriction as for primary parameters, i.e. those having fixed=true. Maybe we should just merge both routines, since we do this kind of checks already for these parameters. That would allow to have parameters depending on external object and vice versa, as long as there is no algebraic loop involved.

We do one such check in the front-end at the moment. It should be done in the backend though. But if you think external objects can be handled by the routine, add it for sure :)

comment:10 Changed 7 years ago by adrpo

I guess one needs to have a look at the FMI stuff too, so that the order is changed there too.

comment:11 Changed 7 years ago by lochel

I've created OMCompiler#1342 for this ticket. It seems that all the examples from the testsuite are working with the changes. Do we have some more test cases for external objects?

comment:12 follow-up: Changed 7 years ago by sjoelund.se

Probably among all the libraries. Try creating a testcase for the tricky case of having a parameter fixed=true depend on an external object you create.

comment:13 in reply to: ↑ 12 Changed 7 years ago by lochel

Replying to sjoelund.se:

Probably among all the libraries.

That's why we should have the option to test all the libraries for certain pull requests...

comment:14 Changed 7 years ago by lochel

  • Owner changed from somebody to lochel
  • Status changed from new to accepted

comment:15 Changed 7 years ago by lochel

The fix is working fine for the c runtime. Before merging this, I will at least try to apply the same changes to the cpp runtime as well.

comment:16 Changed 7 years ago by lochel

I've pushed the changes to the c runtime, which means that the model from the description should work with the next nightly build.

comment:17 Changed 7 years ago by rfranke

This commit fixes the example for the Cpp runtime. The fix is unrelated to the changed initialization of external objects!?

Your new test Add test for ticket:3446 goes beyond this thicket, but makes sense of course.

comment:18 Changed 7 years ago by lochel

  • Milestone changed from Future to 1.12.0

Well, this ticket was easy to fix even without using the sorting. My example has an external object (declared as parameter) that has to be initialized between two other parameters. That will only work with proper sorting of course.

comment:19 follow-up: Changed 7 years ago by rfranke

PR 1353 adapts the initialization of parameters in the Cpp runtime. Now your new test runs with Cpp as well.

comment:20 in reply to: ↑ 19 Changed 7 years ago by lochel

  • Resolution set to fixed
  • Status changed from accepted to closed

Replying to rfranke:

PR 1353 adapts the initialization of parameters in the Cpp runtime. Now your new test runs with Cpp as well.

Thanks :)

Note: See TracTickets for help on using tickets.