#2565 closed defect (fixed)
Problems with code generation - package constants in ExternalMedia
Reported by: | Francesco Casella | Owned by: | Per Östlund |
---|---|---|---|
Priority: | critical | Milestone: | 1.21.0 |
Component: | New Instantiation | Version: | trunk |
Keywords: | Cc: | Adrian Pop |
Description
I am trying to make the ExternalMedia library to run in OMC. The library can be checked out from the MA SVN repo
https://svn.modelica.org/projects/ExternalMediaLibrary/trunk
When I try to compile the model
ExternalMedia.Test.TestMedium.TestBasePropertiesExplicit
I get the following errors from the gcc compiler:
ExternalMedia.Test.TestMedium.TestBasePropertiesExplicit.c:81: error: '$PMedium$PfluidConstants$lB1$rB$PcriticalTemperature' undeclared
ExternalMedia.Test.TestMedium.TestBasePropertiesExplicit.c:90: error: '$PMedium$PfluidConstants$lB1$rB$PcriticalPressure'
etc.
Can you have a look at that? These are all package constants.
Change History (34)
comment:1 by , 11 years ago
Component: | Code Generation → Frontend |
---|---|
Owner: | changed from | to
Status: | new → assigned |
comment:2 by , 11 years ago
I have found a very simple workaround: I just redeclare all those package constants, providing them directly in the package declaration by means of literal Real constants:
package TweakedExternalMedia package TestMedium extends ExternalMedia.Media.TestMedium( externalFluidConstants = FluidConstants( iupacName= "unknown", casRegistryNumber= "unknown", chemicalFormula= "unknown", structureFormula= "unknown", molarMass= 0.028, criticalTemperature= 623.0, criticalPressure= 221e5, criticalMolarVolume= 15, acentricFactor= 0, triplePointTemperature= 280.0, triplePointPressure= 500.0, meltingPoint= 280, normalBoilingPoint= 380.0, dipoleMoment= 2.0)); end TestMedium; model TestStatesSat extends ExternalMedia.Test.TestMedium.TestStatesSat( redeclare package Medium = TweakedExternalMedia.TestMedium); end TestStatesSat; end TweakedExternalMedia;
The model TweakedExternalMedia.TestStatesSat is now flattened correctly. This is not very elegant, because the medium model should get these constants from the external C code, but it is no big deal to temporarily provide this data manually. I'll keep the ticket open, because eventually the front end should take care of this automatically.
comment:3 by , 11 years ago
I don't think is needed to be done like this.
I think it will work if you directly define inside:
ExternalTwoPhaseMedium
String mediumName String libraryName String systemName
and not in the extends.
comment:4 by , 11 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
OK, I removed the redeclaration of mediumName from the extends clause (see r7296 of the svn.modelica.org/projects/ExternalMediaLibrary/trunk repo) and I no longer get the problem I reported.
The only minor issue is that I cannot change the mediumName string compared to the base class PartialTwoPhaseMedium, otherwise I get an error because the elements are not identical. This is no big deal, as this string will be changed in further redeclarations, when the medium-specific strings are set (see, e.g., ExternalMedia.Examples.WaterIF97).
Thank you Adrian for the suggestion!
comment:5 by , 11 years ago
Resolution: | fixed |
---|---|
Status: | closed → reopened |
I'll keep this opened until I really fix the issue.
comment:6 by , 11 years ago
I'm sorry, I fumbled up with the testing. The reason why I din't get the problem reported is that I ran a different test model which did not use package constants. Unfortunately the problem is still there if you try to use those constants.
You can now reproduce the error by using ExternalMedia.Test.TestMedium.TestConstants, which is as simple as possible.
There is one change in the behaviour of the compiler, though. Using yesterday's nightly build (r19175), I get messages = "Failed to build model: ExternalMedia.Test.TestMedium.TestConstants"
, while a previous nightly (r19047) led to the C-code errors originally reported in this ticket. This holds irrespective of the change in the library made in r7296. Apparently the front end is now able to recognize that there is some problem, but it still cannot handle it. BTW, I would suggest to provide a bit more information in the error message.
Anyway, it should be possible to break the circular dependency in the extends clause by splitting the declaration of ExternalTwoPhase medium in two stages, using inheritance. I'll work on that a bit and see what I can do.
comment:7 by , 11 years ago
OK, it seems that the change in ExternalMedia r7296 actually works, but then there is another problem with code generation so the link to the Include annotation is lost for functions which are evaluated at compile time for package constant binding (or something like that), see #2412
Temporary workaround: copy externalmedialib.h
to C:\OpenModelica1.9.1Nightly\include\omc\c
. A fix is already in the pipeline, and eventually also the circular dependency should be handled. Then the ticket will be closed :)
comment:8 by , 11 years ago
Adrian; can you check this on Windows and link the error-messages?
On Linux the following works fine (after I got some hints from Francesco on how to build it):
loadModel(ExternalMedia);getErrorString(); simulate(ExternalMedia.Test.TestMedium.TestConstants);getErrorString();
Gives:
true "" record SimulationResult resultFile = "/home/martin/tmp/ExternalMedia.Test.TestMedium.TestConstants_res.mat", simulationOptions = "startTime = 0.0, stopTime = 1.0, numberOfIntervals = 500, tolerance = 1e-06, method = 'dassl', fileNamePrefix = 'ExternalMedia.Test.TestMedium.TestConstants', options = '', outputFormat = 'mat', variableFilter = '.*', measureTime = false, cflags = '', simflags = ''", messages = "", timeFrontend = 3.160652631, timeBackend = 0.005224445, timeSimCode = 0.009258942000000001, timeTemplates = 0.003776748, timeCompile = 0.351857477, timeSimulation = 0.008373283, timeTotal = 3.539239611 end SimulationResult; ""
comment:9 by , 11 years ago
So for the libraries, I get on Linux:
clang -shared -o ExternalMedia_getMolarMass_0.so ExternalMedia_getMolarMass_0.o ExternalMedia_getMolarMass_0_records.o -Wl,-rpath="/home/martin/.openmodelica/libraries/ExternalMedia 3.2.1/Resources/Library" "-L/home/martin/.openmodelica/libraries/ExternalMedia 3.2.1/Resources/Library" -lExternalMediaLib -I"/home/martin/trunk/build/include/omc/c" "-I/home/martin/.openmodelica/libraries/ExternalMedia 3.2.1/Resources/Include" -fPIC -L"/home/martin/trunk/build/lib/omc" -Wl,-rpath,'/home/martin/trunk/build/lib/omc' -lOpenModelicaRuntimeC -llapack -lblas -lm -rdynamic -lm
While Francesco on Windows gets no includes to the library
comment:10 by , 11 years ago
I used this script:
loadModel(Modelica); getErrorString(); loadFile("ExternalMedia 3.2.1/package.mo"); getErrorString(); simulate(ExternalMedia.Test.TestMedium.TestConstants); getErrorString();
So maybe is loadFile
that has the issue?
comment:11 by , 11 years ago
I saw ==== Log /tmp/omc-rtest-build/openmodelica/typed-API/log-UriLookup.mos in the OM_WIN logs...
This has to work or the include is not used!
uriToFilename("modelica://ExternalMedia/Resources/Include");getErrorString();
comment:13 by , 11 years ago
Or actually... The internal command that does uri lookup has to work. It's different from that API call :(
comment:14 by , 11 years ago
I get
true "" true "" ("D:\\Lavoro\\ModelicaSVN\\ExternalMediaLibrary\\Modelica\\ExternalMedia 3.2.1/Resources/Include","") ""
I guess too many back-forward slashes around...
comment:15 by , 11 years ago
realpath implementation changed a while back in Windows and returns backwards slashes.
I'll see if I can make it return forward slashes as GCC might have issues otherwise.
But I think that the problem is somehow not there as I don't see
any include other the default (include/omc/c) in the makefile or .log.
I'll do some debugging tomorrow.
comment:16 by , 11 years ago
Bug #2412 (with the IncludeDirectory/LibraryDirectory) is now fixed.
I'll still leave this bug open until I commit my changes for better handling of package constants.
comment:17 by , 10 years ago
Tried this with r22673. The model now compiles and simulates successfully, but it takes 4 minutes to compile and it generates over 600 MB of binary stuff, which is totally crazy for such a model. In comparision, Dymola takes 8 seconds to compile it, and generates 6 MB of binaries.
follow-up: 25 comment:19 by , 10 years ago
ExternalMedia.Test.TestMedium.TestBasePropertiesExplicit
, as stated initially.
comment:20 by , 10 years ago
For me the object-files for that are only 2.6MB. And translation+compilation was around 30 seconds. Linking failed due to the library though. Tried reverting it, but ended up with no library. It's a little boring to try recompiling the library for Linux again.
I am fairly sure it should work better than it does for you, but it is hard for me to try debugging it...
comment:21 by , 10 years ago
Cc: | added |
---|
I've retried with r22761 but I still get 79 dlls to compute the damn package constants. All the way from ExternalMedia_getMolarMass_0_records.c/.dll to ExternalMedia_getCriticalMolarVolume_79.c/.dll
I'm not sure if this is Windows-specific, I thought code generation would be platform-independent, thanks to MinGW, but I might be wrong.
As to the library compilation, you can ask Jorrit Wronski (jowr@…) - he's testing the library under Linux so he can help you with that. Maybe he could just commit the library binaries on SVN, though I don't know if static libraries are universal in Linux or rather installation- or version-specific.
comment:22 by , 10 years ago
The simulation executable does not depend on the dll's though. They are temporary ones in order to evaluate constants that depend on an external call during the compilation process.
comment:23 by , 10 years ago
I did not push all my changes into the trunk/ so the way package constants that depend on external functions are handled is the same as before.
The next step is to not evaluate any constant or parameter if they are not structural or have annotation(Evaluate=true) and I'm working on it.
comment:24 by , 10 years ago
Milestone: | 1.9.1 → 1.9.2 |
---|
This ticket was not closed for 1.9.1, which has now been released. It was batch modified for milestone 1.9.2 (but maybe an empty milestone was more appropriate; feel free to change it).
comment:25 by , 10 years ago
Priority: | high → critical |
---|
After the last fix that affected ExternalMedia (r23968) I still get 79 dll's (more than 500 MB) and over four minutes of compilation time when trying to compile the very basic ExternalMedia.Test.TestMedium.TestBasePropertiesExplicit
model under Windows/MinGW.
We have workarounds to avoid this phenomenon for our projects (hard-coding those constants as literals in the Modelica code for each separate medium model), but this is still quite inconvenient and it would be nice to have the problem fixed once and for all.
Is there any progress on this front?
comment:26 by , 10 years ago
Milestone: | 1.9.2 → 1.9.3 |
---|
Milestone changed to 1.9.3 since 1.9.2 was released.
comment:31 by , 8 years ago
Milestone: | 1.11.0 → 1.12.0 |
---|
Milestone moved to 1.12.0 due to 1.11.0 already being released.
comment:32 by , 7 years ago
Component: | Frontend → New Instantiation |
---|---|
Milestone: | 1.12.0 → 2.0.0 |
Owner: | changed from | to
Status: | reopened → assigned |
I am now resuming work on ExternalMedia, see https://github.com/modelica/ExternalMedia. We'll need to see if these problems are solved with the new front end as soon as it is capable to deal with Modelica.Media stuff.
comment:33 by , 2 years ago
Milestone: | 2.0.0 → 1.21.0 |
---|---|
Resolution: | → fixed |
Status: | assigned → closed |
This is front-end and not back-end. That constant should be replaced during flattening.