Model translation warns about failed assignments and fails with -d=dumpSimCode #10519
Comments
|
@mahge, @kabdelhak, mind having a look? What is strange is that the model compilation, execution and verification are all fine. |
|
But it doesn't work with cpp runtime 😞 |
extra case to correctly parse arrays that end up in simple assignments, mainly used for record attributes that are arrays. partially solves ticket OpenModelica#10519
|
Can't reproduce the mentioned behavior that the model runs fine. For me it just fails, also with C-runtime. The problematic part was a weird record assignment. It failed because record attributes that are arrays were not thought of apparently, i fixed it in the PR, now they are parsed correctly, but i still get an error: Multiple redefinitions of functions, i will see what is up here. |
|
Strange. For me the C simulation runs with version 1.0.0 of ThermofluidStream (as can be installed from OMEdit). Using the current master branch: gives |
|
Trying out your PR,
|
|
Oh nevermind, i see now what i did wrong. I think my PR is part of the solution, but there is more to it. |
- came up in discussions on OpenModelica#10519
|
I fixed the broken simcode dump in #10581. It could not handle printing a function pointer being passed to a function, i added the missing cases and also made a backup case of Back on the original problem now. |
extra case to correctly parse arrays that end up in simple assignments, mainly used for record attributes that are arrays. partially solves ticket OpenModelica#10519
* [BE] parse arrays as record attributes extra case to correctly parse arrays that end up in simple assignments, mainly used for record attributes that are arrays. partially solves ticket #10519 * [SimCode] return correct equation index - fix typo * [testsuite] fixed model - remove error msg
- attempts to fix OpenModelica#10519
|
Attempts to fix this problem are in PR #10604 With this PR, the model generates code, but i might have made errors in the code changes. I just added lines of code so that it would first of all not fail during code generation. Running it with the current version of the PR yields following error 4 times: My assumption is, that this error is just the result of the compiler trying to parse an array variable while still having a subscript or dimension indicator at the end. After removing these with dirty changes just to see what happens and if it can actually compile, the following error occurs approx 100 times: Which seems like there is an extra My understanding so far of this problem is that we have records containing arrays, which are tried to be parsed as scalar because it does not expect arrays here (i had to add code to SimCode to make it work). Sine this should be a common structure there is probably something special about this record (individual record variable bindings, special constructor?). As a result the naming of the variables seems to be off. What would be the expected code at the mentioned places? |
|
Thank you for your effort so far @kabdelhak! StatArrayDim1<double, 1, true> __omcQ_24TMP_5FThermofluidStream_5FExamples_5FSimpleAirCycle_5Fsource1_5Foutlet_5FMedium_5FThermodynamicState474_P_X_;
#define __omcQ_24TMP_5FThermofluidStream_5FExamples_5FSimpleAirCycle_5Fsource1_5Foutlet_5FMedium_5FThermodynamicState474_P_X_(2) _pointerToRealVars[61]Only one can be right: either array of size 2 or expanded element with with dumpSimCode says:
A similar confusion, but the other way around happens in Algloop666. The model header file declares an array of size 2: StatArrayDim1<double, 2, true> _junctionT1_1_P_junctionN_P_r_in_;The first element should be accessed with At least the declaration in the model header file is correct in this case. Note that in SimCode the lines for index 268 and 269 end with size |
|
@kabdelhak it looks that you have solved the second problem with Algloop666 already. --- a/OMCppThermofluidStream.Examples.SimpleAirCycle.generated.h
+++ b/OMCppThermofluidStream.Examples.SimpleAirCycle.h
@@ -251,20 +251,16 @@ protected:
StatArrayDim1<double, 1, true> __omcQ_24TMP_5F594_;
StatArrayDim1<double, 1, true> __omcQ_24TMP_5F72_;
#define __omcQ_24TMP_5FThermofluidStream_5FExamples_5FSimpleAirCycle_5Fsource1_5Foutlet_5FMedium_5FThermodynamicState474_P_T_ _pointerToRealVars[59]
- StatArrayDim1<double, 1, true> __omcQ_24TMP_5FThermofluidStream_5FExamples_5FSimpleAirCycle_5Fsource1_5Foutlet_5FMedium_5FThermodynamicState474_P_X_;
- #define __omcQ_24TMP_5FThermofluidStream_5FExamples_5FSimpleAirCycle_5Fsource1_5Foutlet_5FMedium_5FThermodynamicState474_P_X_(2) _pointerToRealVars[61]
+ StatArrayDim1<double, 2, true> __omcQ_24TMP_5FThermofluidStream_5FExamples_5FSimpleAirCycle_5Fsource1_5Foutlet_5FMedium_5FThermodynamicState474_P_X_;
#define __omcQ_24TMP_5FThermofluidStream_5FExamples_5FSimpleAirCycle_5Fsource1_5Foutlet_5FMedium_5FThermodynamicState474_P_p_ _pointerToRealVars[62]
#define __omcQ_24TMP_5FThermofluidStream_5FExamples_5FSimpleAirCycle_5Fsource1_5Foutlet_5FMedium_5FThermodynamicState6_P_T_ _pointerToRealVars[63]
- StatArrayDim1<double, 1, true> __omcQ_24TMP_5FThermofluidStream_5FExamples_5FSimpleAirCycle_5Fsource1_5Foutlet_5FMedium_5FThermodynamicState6_P_X_;
- #define __omcQ_24TMP_5FThermofluidStream_5FExamples_5FSimpleAirCycle_5Fsource1_5Foutlet_5FMedium_5FThermodynamicState6_P_X_(2) _pointerToRealVars[65]
+ StatArrayDim1<double, 2, true> __omcQ_24TMP_5FThermofluidStream_5FExamples_5FSimpleAirCycle_5Fsource1_5Foutlet_5FMedium_5FThermodynamicState6_P_X_;
#define __omcQ_24TMP_5FThermofluidStream_5FExamples_5FSimpleAirCycle_5Fsource1_5Foutlet_5FMedium_5FThermodynamicState6_P_p_ _pointerToRealVars[66]
#define __omcQ_24TMP_5FThermofluidStream_5FExamples_5FSimpleAirCycle_5Fsource_5Foutlet_5FMedium_5FThermodynamicState1_P_T_ _pointerToRealVars[67]
- StatArrayDim1<double, 1, true> __omcQ_24TMP_5FThermofluidStream_5FExamples_5FSimpleAirCycle_5Fsource_5Foutlet_5FMedium_5FThermodynamicState1_P_X_;
- #define __omcQ_24TMP_5FThermofluidStream_5FExamples_5FSimpleAirCycle_5Fsource_5Foutlet_5FMedium_5FThermodynamicState1_P_X_(2) _pointerToRealVars[69]
+ StatArrayDim1<double, 2, true> __omcQ_24TMP_5FThermofluidStream_5FExamples_5FSimpleAirCycle_5Fsource_5Foutlet_5FMedium_5FThermodynamicState1_P_X_;
#define __omcQ_24TMP_5FThermofluidStream_5FExamples_5FSimpleAirCycle_5Fsource_5Foutlet_5FMedium_5FThermodynamicState1_P_p_ _pointerToRealVars[70]
#define __omcQ_24TMP_5FThermofluidStream_5FExamples_5FSimpleAirCycle_5Fsource_5Foutlet_5FMedium_5FThermodynamicState389_P_T_ _pointerToRealVars[71]
- StatArrayDim1<double, 1, true> __omcQ_24TMP_5FThermofluidStream_5FExamples_5FSimpleAirCycle_5Fsource_5Foutlet_5FMedium_5FThermodynamicState389_P_X_;
- #define __omcQ_24TMP_5FThermofluidStream_5FExamples_5FSimpleAirCycle_5Fsource_5Foutlet_5FMedium_5FThermodynamicState389_P_X_(2) _pointerToRealVars[73]
+ StatArrayDim1<double, 2, true> __omcQ_24TMP_5FThermofluidStream_5FExamples_5FSimpleAirCycle_5Fsource_5Foutlet_5FMedium_5FThermodynamicState389_P_X_;
#define __omcQ_24TMP_5FThermofluidStream_5FExamples_5FSimpleAirCycle_5Fsource_5Foutlet_5FMedium_5FThermodynamicState389_P_p_ _pointerToRealVars[74]
#define __omcQ_24cse1_ _pointerToRealVars[75]
#define __omcQ_24cse10_ _pointerToRealVars[76] |
|
Btw. just for information: these arrays are only wrappers around a chunk of variables in the overall void SimpleAirCycle::initAlgVars_0()
{
...
__omcQ_24TMP_5FThermofluidStream_5FExamples_5FSimpleAirCycle_5Fsource1_5Foutlet_5FMedium_5FThermodynamicState474_P_X_
.init(&_pointerToRealVars[60]);
...
}and then directly used with array operations, like assignments: /*
equation index: 474
type: ALGORITHM
$TMP_ThermofluidStream_Examples_SimpleAirCycle_source1_outlet_Medium_ThermodynamicState474 := if not source1.setEnthalpy then ThermofluidStream.Examples.SimpleAirCycle.source1.Medium.ThermodynamicState(source1.p0_par, source1.T0_par, {source1.Xi0_par[1], 1.0 - source1.Xi0_par[1]}) else ThermofluidStream.Examples.SimpleAirCycle.source1.Medium.ThermodynamicState(source1.p0_par, Modelica.Math.Nonlinear.solveOneNonlinearEquation(function ThermofluidStream.Examples.SimpleAirCycle.source1.Medium.T_phX.f_nonlinear(#(source1.p0_par), #(source1.h0_par), #({source1.Xi0_par[1]})), 190.0, 647.0, 1e-13), {source1.Xi0_par[1], 1.0 - source1.Xi0_par[1]});
*/
void SimpleAirCycle::evaluate_474()
{
ThermofluidStream_Examples_SimpleAirCycle_source1_Medium_ThermodynamicStateType tmp1216;
ThermofluidStream_Examples_SimpleAirCycle_source1_Medium_ThermodynamicStateType tmp1218;
Modelica_Math_Nonlinear_solveOneNonlinearEquationRetType tmp1220;
ThermofluidStream_Examples_SimpleAirCycle_source1_Medium_ThermodynamicStateType tmp1222;
if (!_source1_P_setEnthalpy_) {
...
} else {
double tmp1219_data[]={_source1_P_Xi0_par_(1)};
StatArrayDim1<double, 1> tmp1219(tmp1219_data);
_functions->Modelica_Math_Nonlinear_solveOneNonlinearEquation(_Closure23_ThermofluidStream_Examples_SimpleAirCycle_source1_Medium_T__phX_f__nonlinear(_functions, /*box double*/_source1_P_p0_par_, /*box double*/_source1_P_h0_par_, /*box BaseArray<double>*/tmp1219), 190.0, 647.0, 1e-13, tmp1220);
double tmp1221_data[]={_source1_P_Xi0_par_(1), (1.0 - _source1_P_Xi0_par_(1))};
StatArrayDim1<double, 2> tmp1221(tmp1221_data);
tmp1218.p_ = _source1_P_p0_par_;
tmp1218.T_ = tmp1220;
tmp1218.X_ = tmp1221;
tmp1222 = tmp1218;
}
__omcQ_24TMP_5FThermofluidStream_5FExamples_5FSimpleAirCycle_5Fsource1_5Foutlet_5FMedium_5FThermodynamicState474_P_p_ = tmp1222.p_;
__omcQ_24TMP_5FThermofluidStream_5FExamples_5FSimpleAirCycle_5Fsource1_5Foutlet_5FMedium_5FThermodynamicState474_P_T_ = tmp1222.T_;
__omcQ_24TMP_5FThermofluidStream_5FExamples_5FSimpleAirCycle_5Fsource1_5Foutlet_5FMedium_5FThermodynamicState474_P_X_
.assign(tmp1222.X_);
}Assignment of arrays with A pity that we still have separate equations for assignments of individual array elements in the SimCode, like: /*
equation index: 477
type: SIMPLE_ASSIGN
splitterT1_1.outletA.state.X[1] = $TMP_ThermofluidStream_Examples_SimpleAirCycle_source1_outlet_Medium_ThermodynamicState474.X[1]
*/
void SimpleAirCycle::evaluate_477()
{
_splitterT1_1_P_outletA_P_state_P_X_(1) = __omcQ_24TMP_5FThermofluidStream_5FExamples_5FSimpleAirCycle_5Fsource1_5Foutlet_5FMedium_5FThermodynamicState474_P_X_(1);
}
/*
equation index: 478
type: SIMPLE_ASSIGN
splitterT1_1.outletA.state.X[2] = $TMP_ThermofluidStream_Examples_SimpleAirCycle_source1_outlet_Medium_ThermodynamicState474.X[2]
*/
void SimpleAirCycle::evaluate_478()
{
_splitterT1_1_P_outletA_P_state_P_X_(2) = __omcQ_24TMP_5FThermofluidStream_5FExamples_5FSimpleAirCycle_5Fsource1_5Foutlet_5FMedium_5FThermodynamicState474_P_X_(2);
} |
This addresses OpenModelica#10519 -- Model translation warns about failed assignments and fails with -d=dumpSimCode.
|
PR #10630 adds the missing array dimensions. This seems to have been the root cause as the issue disappears without further changes. |
This addresses #10519 -- Model translation warns about failed assignments and fails with -d=dumpSimCode.
…mCode This should be handled in the following template phase more efficiently. The given comment might be historic: "The routines generate bad code for arrays inside the record unless we expand them" See also example in issue OpenModelica#10519.
|
PR #10632 changes the above assignment of individual array elements to a combined /*
equation index: 474
type: SIMPLE_ASSIGN
splitterT1_1.outletA.state.X = $TMP_ThermofluidStream_Examples_SimpleAirCycle_source1_outlet_Medium_ThermodynamicState471.X
*/
void SimpleAirCycle::evaluate_474()
{
//Array assign
_splitterT1_1_P_outletA_P_state_P_X_ = __omcQ_24TMP_5FThermofluidStream_5FExamples_5FSimpleAirCycle_5Fsource1_5Foutlet_5FMedium_5FThermodynamicState471_P_X_;
}... and Jenkins shows that it does not work for all cases :( |
|
... the generated assignments don't mach in all cases without expansion. Expanding everything matches them brute force. But that's another issue. This one is solved with PR #10630. |
|
Thank's to the work by @kabdelhak on dumpSimCode, we could better analyse the problem and then solve it. |
Description
The model
ThermofluidStream.Examples.SimpleAirCycle, as well as 11 more models of that library, raise warnings of the form:The translation fails when
-d=dumpSimCodeis specified.Equations like
might better be formulated as
to fit into
SimCodeUtil.makeSES_SIMPLE_ASSIGNand to improve the generated code (no need to create a temporary array{X[1],X[2]}for the lhs when directly assigning toX).Steps to Reproduce
Simulate the model to get the warnings. Alternatively the smaller
ThermofluidStream.Processes.Tests.NozzleSpecify
-d=dumpSimCodeto make the translation fail.Expected Behavior
The models should translate without warning and dumpSimCode should work.
Version and OS
The current master branch.
The text was updated successfully, but these errors were encountered: