Opened 10 years ago
Closed 10 years ago
#3263 closed defect (fixed)
Cpp runtime generates wrong code for MSL matrix function
Reported by: | Rüdiger Franke | Owned by: | Rüdiger Franke |
---|---|---|---|
Priority: | high | Milestone: | 1.9.3 |
Component: | Run-time | Version: | trunk |
Keywords: | Cc: | Marcus Walther, Volker Waurich, Niklas Worschech |
Description
The Cpp runtime generates wrong code for the matrix function Modelica.Math.Matrices.sort
. The following model:
model SortTest2 input Real u; // prevent evaluation by frontend Real[:, :] M = [3, 4; 1 + u, 2]; Real[size(M, 1), size(M, 2)] N; equation N = Modelica.Math.Matrices.sort(M); annotation(uses(Modelica(version="3.2.1"))); end SortTest2;
results in a strange assignment to the function input M and an unexpanded SMT_ASSIGN_ARR:
OMCppSortTest2Functions.cpp: In member function ‘void Functions::Modelica_Math_Matrices_sort(BaseArray<double>&, bool, bool, Modelica_Math_Matrices_sortRetType&)’: OMCppSortTest2Functions.cpp:210:47: error: expected primary-expression before ‘.’ token sorted_M.setDims(tmp3,tmp4);/*setDims 1*/M = .targTest91 ^ OMCppSortTest2Functions.cpp:266:10: error: ‘STMT_ASSIGN_ARR’ was not declared in this scope STMT_ASSIGN_ARR RANGE ^
Change History (7)
comment:2 by , 10 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
r25403 introduces array slices to solve the lhs assignment problem. Using the new templates algStmtAssignArrCref
and daeExpCrefIndexSpec
, the above assignment to sorted_M
is now translated to:
vector<Slice> slice; slice.push_back(Slice(j + gap + 1)); // index slice.push_back(Slice()); // whole dim ArraySlice<double>(sorted_M, slice) = wM2;
The SortTest2 example worked after having also fixed the rhs problem in my working copy temporarily!
ArraySlice basically inverts the function create_array_from_shape
and it pre-implements the treatment of index sets that is generated explicitly by daeExpCrefRhsIndexSpec
.
Ultimately the array slices should also be applicable to rhs, making daeExpCrefRhsIndexSpec
and create_array_from_shape
obsolete, possibly avoiding some copy operations. But this change needs some more thoughts, because it could break already working things.
comment:3 by , 10 years ago
rfranke, you forgot to copy the new header file to the build/include directory:
https://test.openmodelica.org/hudson/job/OpenModelica_TEST_CLANG/10720/
/run/shm/OpenModelica_BUILD_CLANG/OpenModelica-clang/build/include/omc/cpp/Core/Modelica.h:198:10: fatal error: 'Core/Math/ArraySlice.h' file not found #include <Core/Math/ArraySlice.h>
comment:5 by , 10 years ago
r25443 introduces ArraySlice for rhs expressions as well, along with some cleanup in Array.h in r25432, r25436, r25438, r25439, and a new getDim method in r25437.
The template daeExpCrefRhsIndexSpec
and the function create_array_from_shape
are not used anymore. The former has at least two bugs: it forgets to clear index sets if used in a loop and WHOLEDIM does not work. It should be removed from CodegenCpp.tpl
once the new implementation has proven better.
The Modelica statement
sorted_M[j + 1, :] := sorted_M[j + gap + 1, :];
now is translated to:
tmp43.clear(); tmp43.push_back(Slice((1 + (j + gap)))); tmp43.push_back(Slice()); ArraySlice<double> tmp43_as(sorted_M, tmp43); tmp44.clear(); tmp44.push_back(Slice((1 + j))); tmp44.push_back(Slice()); ArraySlice<double>(sorted_M, tmp44).assign(tmp43_as);
The generated Cpp code creates no index sets or temporary copies -- its all up to the Array(Slice) implementation. Even though the current version still does a lot internally, it could be optimized without comprising the generated Cpp code.
comment:6 by , 10 years ago
r25567 treats the reported targTest91 problem by not calling the generating template varDefaultValue anymore if already some exp is assigned as default value to an array. The template is still called in the else branch.
comment:7 by , 10 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
r25402 solves the reported compiler errors.
The code for
.targTest9<%i%>
is not generated anymore if there is no<%outStruct%>
before the dot -- don't know if this fixes the root cause; lacking an idea about the intention oftargTest
.The implementation of STMT_ASSIGN_ARR RANGE appears straightforward when looking at the already existing STMT_ASSIGN_ARR CALL in the same template algStmtAssignArr.
The compiled model generates wrong results though. There are deficiencies in the code generation for array slices, both rhs and lhs: