#6331 closed defect (fixed)
[NF] Assigning parameters by external functions
Reported by: | Owned by: | Per Östlund | |
---|---|---|---|
Priority: | high | Milestone: | NeedsInput |
Component: | New Instantiation | Version: | 1.16.0 |
Keywords: | NONSTRUCTURAL_PARAMETER | Cc: |
Description
By the pull request by Perost, the external functions are now able to assign values to parameters "https://github.com/OpenModelica/OpenModelica/pull/210".
Overall, it works well.
However, the arrays of parameter (record) never work!
In the following example, the model "test5" shows such a problem with myParamsArray and myParamsArray2; actually extracting the elements are not possible giving some errors in the compilation.
This investigation is motivated from an attempt to fix the "ExternalMedia" library; the constant (fluidConstants) inherited by Modelica.Media is assigned by some external functions, which gives rise to a trouble relevant to the issue of NFEvalFunction.evaluateExternal (https://trac.openmodelica.org/OpenModelica/ticket/5240).
I'm just trying to circumvent the error by replace the constant "externalFluidConstants" with a parameter of the same contents as a temporary remedy. However, the final replacement of fluidConstants should be an array with 1 element. So I get stuck; this is the same matter of the previous struggle with the old front end, when I abandoned fluidConstants using externalFluidConstants instead..
package TestEvalExt record MyRecord "a record whose members are assigned by an external function" Real p1 = extfunc1(), p2 = 2. * extfunc1(); end MyRecord; parameter MyRecord myParams(p1 = 0.1 * extfunc1(), p2 = extfunc1()); parameter MyRecord[1] myParamsArray = {myParams}; parameter MyRecord[1] myParamsArray2; model test1 "initialzing a constant by an external function (NOT WORKING!)" constant MyRecord params(p1 = 0.1 * extfunc1(), p2 = extfunc1()); //<-- it's a constant assigned by an external function.. end test1; model test2 "initialzing a parameter by an external function (WORKING!)" parameter MyRecord params(p1 = 0.1 * extfunc1(), p2 = extfunc1()); //<-- it's a parameter assigned by an external function.. end test2; model test3 "initialzing a parameter and an array (WORKING!)" parameter MyRecord params(p1 = 0.1 * extfunc1(), p2 = extfunc1()); // <-- a parameter parameter MyRecord[2] params_array = {params, params}; // <-- a array of the parameter end test3; model test4 "getting a subelement from the initialized parameter (WORKING!)" extends test3; parameter Real p1 = params_array[2].p1; //from an inherited array parameter Real p2 = myParams.p2; //from a parameter in the package (initialized outside..) end test4; model test5 "getting a subelement from the parameter array (NOT WORKING!)" extends test3; parameter Real p2 = myParamsArray[1].p2; // <-- from an array of paramater of the package (initialized outside..) parameter Real p2 = myParamsArray2[1].p2; // <-- it doesn't work too.. end test5; function extfunc1 output Real ret; external "C" annotation( Include = " double extfunc1(void){ return 1.234; } "); end extfunc1; end TestEvalExt;
Attachments (1)
Change History (17)
by , 4 years ago
Attachment: | TestEvalExt.mo added |
---|
comment:1 by , 4 years ago
Keywords: | NONSTRUCTURAL_PARAMETER added; STRUCTURAL_PARAMETER removed |
---|
comment:2 by , 4 years ago
The issue with test5 is that we don't expand the package parameters properly, so the subscripts on the package parameters we add to the model are lost. This should be fixed in PR 7065.
test1 still doesn't work, but that probably doesn't matter in this case since it's not allowed to have constants with impure functions as bindings. External functions are impure by default, and while extfunc1
is technically pure in this case I assume the real functions in the library aren't.
comment:3 by , 4 years ago
The PR is now merged. Please check if this fixes your issue in the library too.
comment:4 by , 4 years ago
@spinhalf, you can download tomorrow's 1.17.0 nightly build and try with that.
follow-up: 6 comment:5 by , 4 years ago
Thank you!
However, the issue is still pending in spite of a certain progress on the previous test case.
I found the actual spot of trouble in "fluidConstants" of the "ExternalMedia" library is located on a package, i.e. Medium, as a member of the target model; the previous one was referring a record of a package in the model which is 'a member of the mother package'.
So, I just simplify the problem again, as the following code, expecting the same progress to the previous time; I hope I really picked the actual spot different from the previous near-miss (sorry..)
package TestEvalExt2 package MyPackage replaceable record MyRecord Real p; end MyRecord; parameter MyRecord myParams(p = extfunc1()); parameter MyRecord[1] myParamsArray = {myParams}; function extfunc1 output Real ret; external "C" annotation(Include = " double extfunc1(void){ return 1.234; } "); end extfunc1; end MyPackage; model test1 "getting a subelement from the parameter record array (NOT WORKING!)" package Package = MyPackage; parameter Real p = Package.myParams.p "Gives compilation error!!"; end test1; model test2 "getting a subelement from the parameter record (NOT WORKING!)" package Package = MyPackage; parameter Real p = Package.myParamsArray[1].p "Gives a wrong result (not intialized properly)!!"; end test2; end TestEvalExt2;
comment:6 by , 4 years ago
Replying to spinhalf@…:
So, I just simplify the problem again, as the following code, expecting the same progress to the previous time; I hope I really picked the actual spot different from the previous near-miss (sorry..)
Fixed in 42cf5d60, both the test models now simulate successfully for me.
follow-up: 8 comment:7 by , 4 years ago
Great! they work!
So, I tested the FluidConstants record in the ExternalMedia library, but I found another problem in the translation stage, from the elements of record newly introduced using the "redelare" keyword.
At this state, my temporary measure can be applied to the ExternalMedia test, which is adhoc modification of the main library (the Media library) just for my temporary usage. However, this issue looks critical to fix the ExternalMedia package; I guess I 'm able to discuss with Casella to update ExternalMedia after to the successful test on this problem. Many thanks!!
package TestEvalExt2 package MyPackage replaceable record MyRecord Real p; end MyRecord; parameter MyRecord myParams(p = extfunc1()); parameter MyRecord[1] myParamsArray = {myParams}; function extfunc1 output Real ret; external "C" annotation( Include = " double extfunc1(void){ return 1.234; } "); end extfunc1; end MyPackage; model test1 "getting a subelement from the parameter record array (NOW WORKING!)" package Package = MyPackage; parameter Real p = Package.myParams.p "Gives compilation error!!"; end test1; model test2 "getting a subelement from the parameter record (NOW WORKING!)" package Package = MyPackage; parameter Real p = Package.myParamsArray[1].p "Gives a wrong result (not intialized properly)!!"; end test2; model test3 "a new trouble of parameters with the replaced package (NOT WORKING)" package Package extends MyPackage; redeclare record MyRecord Real p1, p2; end MyRecord; parameter MyRecord myParams(p1 = extfunc1(), p2 = 0.1 * extfunc1()); parameter MyRecord[1] myParamsArray = {myParams}; end Package; parameter Real p = Package.myParamsArray[1].p1 "This gives translation error"; end test3; end TestEvalExt2;
comment:8 by , 4 years ago
Replying to spinhalf@…:
So, I tested the FluidConstants record in the ExternalMedia library, but I found another problem in the translation stage, from the elements of record newly introduced using the "redelare" keyword.
The redeclare is invalid since the MyRecord
record you're redeclaring with doesn't have a p
component, so it's not a subtype of the record you're replacing. The new frontend does not yet give good error messages for such issues.
It works if you instead do redeclare record extends MyRecord
though.
comment:9 by , 4 years ago
Oops, thank you!!
That was a confusion during the simplifications of the actual issue, i.e. the 'fluidConstants' parameter initialization of the 'ExternalMedia' package. However, the same parameters in the parent record still make me confused as I show you with a revised test model. So, would you mind advising me once more?
Furthermore, I see the last hurdle in the test models of the 'ExternalMedia' package related the replaced 'Medium'. I just try to reproduce the same compilation error of a missing link to the replaced package, and I would like to ask your effort; I guess we are almost close to the target;
Again, thank you very much!!
package TestEvalExt3 partial package MyPackage replaceable record MyRecord Real p; end MyRecord; //parameter MyRecord myParams(p = extfunc1()) "comment out (see 'Real p' in the model 'test' )"; //parameter MyRecord[1] myParamsArray = {myParams} "comment out at the same"; function extfunc1 output Real ret; external "C" annotation( Include = " double extfunc1(void){ return 1.234; } "); end extfunc1; end MyPackage; model Submodel "a model to be an element of 'test'" package member = MyPackage; Real p = member.myParamsArray[1].p1; end Submodel; model test "a new trouble with replaceble parameter (NOT WORKING)" package Package extends MyPackage; redeclare record extends MyRecord Real p1, p2; end MyRecord; parameter MyRecord myParams(p1 = extfunc1(), p2 = 0.1 * extfunc1()); parameter MyRecord[1] myParamsArray = {myParams}; end Package; Real p = Package.myParamsArray[1].p1 "This works, only if 'MyPackage' (the parent) doesn't have the parameter elements, 'myParams' and 'myParamsArray', which I commented out."; Submodel myModel(redeclare package member = Package) "This doesn't work, which mimics assigning a 'Meidum', for instance, to a model of fluid component."; end test; end TestEvalExt3;
comment:10 by , 4 years ago
I should have thought of this earlier, but packages are not actually allowed to contain parameters, only constants (see 4.6). I could probably fix the last issue, but then it'll all just break once we implement some more rigorous checks for class specializations.
So I guess it's better to focus on fixing #5240 so you can use constants instead.
follow-up: 12 comment:11 by , 4 years ago
Thank you for such an immediate response!
It's a good point, because I just considered the parameters as an alternative.
Indeed, I just want to avoid the issue of #5240, when the "fluidConstants" array is initialized by the external functions which simply gives constants depending on the name of the medium and the actual substance (string constants).
So, under the circumstance, do you estimate that it is possible soon to initialize the constants using such an external function? If so, I guess we can move this issue as a part of #5240.
comment:12 by , 4 years ago
Replying to spinhalf@…:
So, under the circumstance, do you estimate that it is possible soon to initialize the constants using such an external function? If so, I guess we can move this issue as a part of #5240.
We will try to go with the dlopen + libffi route that was suggested in #5240. It'll probably take more time to implement than just porting the old solution we have since it's unexplored territory, but hopefully it won't take too long.
comment:13 by , 4 years ago
#5240 is now mostly fixed, see my comment on that ticket. Please test it out and see if it solves your issues.
comment:14 by , 4 years ago
After all, I confirm that the actual trouble with "constant initialization by external functions" is now cleared for the ExternalMedia library whose spot of error 'was' the fluidConstant constant array's members initialized by the external functions. I tested it with the windows version built by mingw64 clang++ from the sources committed in 03-May-2021, and most of the test runs are successful; thank you very much!! That means the effort of #5240 just solved the trouble directly, and the original issue is now nothing to do with this ticket which is about parameters considered as a means to bypass. Lastly, I have to mention that the ExternalMedia library must be arranged to generate the corresponding DLL in addition to the static library to exploit the new feature of NF, but this is another matter which belong to the workflow for the ExternalMedia library; I'll update and test, in particular, for the interoperatibility with the newest CoolProp and the ThermoPower library.
comment:15 by , 4 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
Great, I'll consider this ticket fixed then.
comment:16 by , 4 years ago
@spinhalf, we've just merged a loot of cool stuff (pun intended) on the master branch of ExternalMedia. I'll update all information there, hopefully today. There are still some issues with OMC on Windows, maybe you can help with that, let's discuss them on the ExernalMedia github issue tracker.
the same code in the description