Opened 4 years ago
Closed 4 years ago
#6089 closed defect (fixed)
Evaluate reductions only involving literal constants in the NF
Reported by: | Francesco Casella | Owned by: | Per Östlund |
---|---|---|---|
Priority: | high | Milestone: | 1.16.0 |
Component: | New Instantiation | Version: | |
Keywords: | Cc: | Martin Sjölund, Karim Adbdelhak, Philip Hannebohm |
Description
@perost, please check out the FEMBody package from https://github.com/looms-polimi/FEMPackage. I just re-sent you an invitation to access the repo.
Please open the FemPackage3.2.mo file and flatten the model ExamplesFEM.PendulumBody
.
The FEMBody model which is instantiated in this example contains code such as
transpose(Cr) = data.inv4 + sum(data.inv5[i, :, :] * q[i] for i in 1:M); Jbar = data.inv7 - sum((transpose(data.inv8[i, :, :]) + data.inv8[i, :, :]) * q[i] for i in 1:M) - sum(sum(data.inv9[i, j, :, :] * q[j] for j in i:M) * q[i] for i in 1:M); [mDcbartilde, Jbar, transpose(Cr)] * [aa_a - g_0; za_a; ddq] = h_w_theta + h_e_theta; [data.Ct, Cr, Me] * [aa_a - g_0; za_a; ddq] = h_w_f + h_e_f - matrix(data.Ke * q) - matrix(d / 100 * (alpha * Me + beta * data.Ke) * dq);
which comes straight from tensor equations describing the multibody FEM model of a flexible link, see paper.
The parameter record data
, which has Evaluate = true
contains multi-dimensional arrays (tensors) of structural parameters of the flexible body, which come from modal analysis of the original FEM model. Most of the elements of those arrays are zero, so for efficient and robust handling of the model equations by the backend it is absolutely essential that the NF reduces the complexity of the model as much as possible without passing it over to the backend, by fully evaluating those expressions.
After fixing #6067, I don't see any surviving parameter name in the flat code, which is good. However, there are several huge reduction expressions, involving the sum operator and literal constant arrays, that are not evaluated and passed straight to the backend. The backend can eventually handle the test case in question, but it produces numerically unreliable code for larger examples.
@perost, could you make sure that these reduction are properly evaluated, so that the backend receives simplified code for further processing?
Change History (5)
comment:2 by , 4 years ago
Replying to perost:
Fixed in 9fe6151. The NF now expands all sum/product reduction expressions with iteration ranges of known size. I'm not sure that's always a good idea
Until we have a scalar backend, I guess so. With the vectorized backend, maybe not, if the iterator range is large.
From what I understand, the only compelling reason to evaluate these reductions is when there is a significant number of zero elements, so that the evaluated expression turns out to be actually simpler than the original reduction. This is often the case when you have models that are written in matrix form for compactness, using matrices that have mostly zero literal elements.
I also added a simplification rule for vector calls with fully expanded arrays as arguments, for example
vector({{x}, {y}, {z}}) => {x, y, z}
, since I noticed the model has quite a lot of those.
Sounds good.
I'll try this with our flexible multibody models as soon as the nightly becomes available.
comment:3 by , 4 years ago
Cc: | added |
---|
Adding Karim and Philip to the discussion as discussed in today's devmeeting. I also invited them to the repository on GitHub
comment:4 by , 4 years ago
Milestone: | 1.17.0 → 1.16.0 |
---|
comment:5 by , 4 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
This issue has been fixed, the PendulumBody
model runs fine.
Fixed in 9fe6151. The NF now expands all sum/product reduction expressions with iteration ranges of known size. I'm not sure that's always a good idea, but we can always add conditions later if needed. We'll see that the library coverage testing says.
I also added a simplification rule for vector calls with fully expanded arrays as arguments, for example
vector({{x}, {y}, {z}}) => {x, y, z}
, since I noticed the model has quite a lot of those.