Opened 14 years ago

Closed 7 years ago

#1452 closed defect (fixed)

Better simplification needed

Reported by: Per Östlund Owned by: Per Östlund
Priority: high Milestone: 1.12.0
Component: Instantiation Version: trunk
Keywords: Cc: Per Östlund

Description

Some parts of ExpressionSimplify needs to be improved, since they are extremely slow. This sample model is extracted from the Modelica.Media.Water.IF97_Utilities.Basic.g1 function, and takes at least 10 minutes to flatten (it might take a lot more than that, I shut it down after 10 min):

record GibbsDerivs
  "derivatives of dimensionless Gibbs-function w.r.t dimensionless pressure and temperature"

  Real pi(unit="1") "dimensionless pressure";
  Real tau(unit="1") "dimensionless temperature";
  Real g(unit="1") "dimensionless Gibbs-function";
  Real gpi(unit="1") "derivative of g w.r.t. pi";
  Real gpipi(unit="1") "2nd derivative of g w.r.t. pi";
  Real gtau(unit="1") "derivative of g w.r.t. tau";
  Real gtautau(unit="1") "2nd derivative of g w.r.t tau";
  Real gtaupi(unit="1") "mixed derivative of g w.r.t. pi and tau";
end GibbsDerivs;

model m
  Real tau2;
  Real o[55];
  GibbsDerivs g;
algorithm
  g.g := g.pi*(-0.00177317424732130 + o[9] + g.pi*(tau2*(-0.000033032641670203
           + (-0.000189489875163150 + o[1]*(-0.0039392777243355 + (-0.043797295650573
           - 0.0000266745479140870*o[13])*o[2]))*tau2) + g.pi*(
          2.04817376923090e-8 + (4.3870667284435e-7 + o[1]*(-0.000032277677238570
           + (-0.00150339245421480 - 0.040668253562649*o[13])*o[2]))*tau2 + g.
          pi*(g.pi*(2.29220763376610e-6*o[14] + g.pi*((-1.67147664510610e-11 +
          o[15]*(-0.00211714723213550 - 23.8957419341040*o[16]))*o[2] + g.pi*(-5.9059564324270e-18
           + o[17]*(-1.26218088991010e-6 - 0.038946842435739*o[18]) + g.pi*(o[
          11]*(1.12562113604590e-11 - 8.2311340897998*o[19]) + g.pi*(
          1.98097128020880e-8*o[15] + g.pi*(o[10]*(1.04069652101740e-19 + (-1.02347470959290e-13
           - 1.00181793795110e-9*o[10])*o[20]) + o[23]*(o[13]*(-8.0882908646985e-11
           + 0.106930318794090*o[24]) + o[21]*(-0.33662250574171*o[26] + o[21]*
          (o[27]*(8.9185845355421e-25 + (3.06293168762320e-13 -
          4.2002467698208e-6*o[15])*o[28]) + g.pi*(-5.9056029685639e-26*o[24]
           + g.pi*(3.7826947613457e-6*o[29] + g.pi*(-1.27686089346810e-15*o[30]
           + o[31]*(7.3087610595061e-29 + o[18]*(5.5414715350778e-17 -
          9.4369707241210e-7*o[32]))*g.pi)))))))))))) + tau2*(-7.8847309559367e-10
           + (1.27907178522850e-8 + 4.8225372718507e-7*tau2)*tau2))))) + (-0.0056087911830200
           + g.tau*(0.071452738814550 + g.tau*(-0.40710498239280 + g.tau*(
          1.42408197144400 + g.tau*(-4.3839511194500 + g.tau*(-9.6927686002170
           + g.tau*(10.0866556801800 + (-0.284086326077200 + 0.0212684635330700
          *g.tau)*g.tau) + log(g.pi)))))))/(o[34]*g.tau);
end m;

The problem is in ExpressionSimplify.simplify2 which calls simplifyBinarySortConstants, which in turns calls simplify2 on all terms and so on. Combined with Static.elabExp calling simplify on each binary expression this scales extremely badly. I have commented out the recursive calls to simplify2 in simplifyBinarySortConstants and simplifyBinarySortConstantsMul for now (see commit 8252), which gets rid of the immediate scalability problems. This causes some minor non-critical changes to the test suite, so it does not seem to be a big issue to remove these calls. But the simplification should probably be rewritten to only simplify an expression once. This means either that simplify should not simplify sub-expressions, or that simplify should not be called on sub-expressions when elaborated.

Update: 10 min might have been a bit of an understatement. I let it run during the night, so the new lower bound on the running time is now 19 hours (it was still not finished, and I couldn't let it run any longer).

Change History (2)

comment:1 by Dietmar Winkler, 9 years ago

Cc: perost, → perost
Milestone: Future

comment:2 by Francesco Casella, 7 years ago

Milestone: Future1.12.0
Resolution: fixed
Status: newclosed

This model is instantiated in a much less than a second with OMC 1.12.0-beta3. I guess the underlying issue was solved long time ago, since the IF97 model has been usable for years.

Note: See TracTickets for help on using tickets.