modelica / ModelicaStandardLibrary

Free (standard conforming) library to model mechanical (1D/3D), electrical (analog, digital, machines), magnetic, thermal, fluid, control systems and hierarchical state machines. Also numerical functions and functions for strings, files and streams are included.
https://doc.modelica.org
BSD 3-Clause "New" or "Revised" License
454 stars 165 forks source link

regFun3 does not always define its outputs #848

Closed modelica-trac-importer closed 7 years ago

modelica-trac-importer commented 7 years ago

Reported by sjoelund.se on 1 Oct 2012 15:02 UTC In Modelica.Fluid.Utilities.regFun3, the c output variable looks like it is not always defined (if y1=y0). The following is a snippet of what the instantiated code looks like.

function Modelica.Fluid.Utilities.regFun3
  ...
  output Real c \"Slope of linear section between two cubic polynomials or dummy linear section slope if single cubic is used\";
  ...
algorithm
  assert(x0 < x1, ...);
  h0 := x1 - x0;
  Delta0 := (y1 - y0) / h0;
  if abs(Delta0) <= 0.0 then
    y := y0;
  else
    ...
    if ... then
      c := ...;
    else
      c := ...;
    end if;
  end if;
end Modelica.Fluid.Utilities.regFun3;

If it is intended that y1<>y0, add an assertion and remove the if-condition and the first branch. Else add some value for c (even if it is not meaningful, make it deterministic).


Migrated-From: https://trac.modelica.org/Modelica/ticket/848

modelica-trac-importer commented 7 years ago

Comment by hansolsson on 1 Oct 2012 16:07 UTC There is also a more subtle issue with that if-branch; the function specifies smoothOrder=1 which is intended to allow AD (algorithmic/automatic differentation).

However, AD is known to fail for singular points - i.e. exactly tests such as this.

modelica-trac-importer commented 7 years ago

Comment by hansolsson on 17 Dec 2012 13:50 UTC This should probably be a blocker; note that to handle the AD-issue we should probably do something like:

if abs(Delta0) <= 0.0 then
  y:=y0+Delta0*firstValue;
  c:=secondValue;
else

with suitable values for firstValue and secondValue.

Yes, for Delta0=0 we will get y:=y0, the point of firstValue is to allow AD to compute the derivative of y.

modelica-trac-importer commented 7 years ago

Comment by hansolsson on 17 Dec 2012 13:57 UTC Change to blocker, as discussed at phone-meeting for MAP.

modelica-trac-importer commented 7 years ago

Modified by fcasella on 5 Mar 2013 23:31 UTC

modelica-trac-importer commented 7 years ago

Comment by anonymous on 7 Mar 2013 08:31 UTC Fixed in 00c266436ebb01175b026fa71dce9021b4d34d6d

I chose to maintain the if-branch due to backward compatibility. Thus, I added the return value for c as suggested by Martin S.

Also, I followed Hans suggestion with firstValue = x-x0 and secondValue=0.

modelica-trac-importer commented 7 years ago

Modified by msielemann on 7 Mar 2013 08:31 UTC

modelica-trac-importer commented 7 years ago

Comment by dietmarw on 7 Mar 2013 08:40 UTC Michael, please remove the <p>...</p> around any other environment (e.g., <ul>) since this is illegal HTML and do not use the Dymola info editor for this.

modelica-trac-importer commented 7 years ago

Comment by msielemann on 7 Mar 2013 08:49 UTC Fixed in f3c96ebaa1dc7606de628f55e954152dbc62ca63