Opened 9 years ago
Closed 8 years ago
#3446 closed defect (fixed)
Impure file name passing breaking CombiTimeTable
Reported by: | Owned by: | Lennart Ochel | |
---|---|---|---|
Priority: | normal | Milestone: | 1.12.0 |
Component: | Backend | Version: | trunk |
Keywords: | impure | Cc: | Lennart Ochel |
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)
Change History (22)
by , 9 years ago
by , 9 years ago
comment:1 by , 9 years ago
Cc: | added; removed |
---|---|
Component: | Unknown → 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 by , 8 years ago
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 by , 8 years ago
comment:4 by , 8 years ago
@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 by , 8 years ago
@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.
follow-up: 8 comment:6 by , 8 years ago
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 by , 8 years ago
Note that there has also been talk in the MA to allow algebraic loops for some external objects (and a re-initilize function).
follow-up: 9 comment:8 by , 8 years ago
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 by , 8 years ago
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 by , 8 years ago
I guess one needs to have a look at the FMI stuff too, so that the order is changed there too.
comment:11 by , 8 years ago
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?
follow-up: 13 comment:12 by , 8 years ago
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 by , 8 years ago
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 by , 8 years ago
Owner: | changed from | to
---|---|
Status: | new → accepted |
comment:15 by , 8 years ago
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 by , 8 years ago
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 by , 8 years ago
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 by , 8 years ago
Milestone: | Future → 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.
follow-up: 20 comment:19 by , 8 years ago
PR 1353 adapts the initialization of parameters in the Cpp runtime. Now your new test runs with Cpp as well.
comment:20 by , 8 years ago
Resolution: | → fixed |
---|---|
Status: | accepted → closed |
ASCII matrix for example