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
476 stars 169 forks source link

Use of Modelica.Constants.eps in Modelica.Fluid.Utilities.regFun3 #3628

Open carlj-w opened 4 years ago

carlj-w commented 4 years ago

Related to #2056.

The function Modelica.Fluid.Utilities.regFun3 uses Modelica.Constants.eps in several places, in assert conditions and if conditions. I am here mostly concerned about the use in if conditions, where the constant is typically multiplied by 100.

At least for me, using the actual eps value for double precision floating point number (approximately 1.11e-16) makes these models not work:

Modelica.Constant.eps is set to ModelicaServices.Machine.eps, which is set to 1e-15 in a non-configured ModelicaServices. Using this value makes the models above work for me.

This seems somewhat problematic. Does anyone know how the value of 100 * Modelica.Constants.eps was reached? Could someone verify my findings, and if so, what should we do? Replace Modelica.Constants.eps by 1e-15 in the function?

HansOlsson commented 4 years ago

I don't how the value was reached, but some possibilities are:

We should also clear up the code, because currently it is hard to see the impact of the tests. Consider:

 elseif abs(y1d + y0d - 2*Delta0) < 100*Modelica.Constants.eps then
...
   c := 3*(y0d + y1d - 2*Delta0)*
 ...
 xstar := .../(-y0d - y1d + 2*Delta0);
...
    omega := 3*(y0d + y1d - 2*Delta0)*

I don't see why the same expressions (except sign) is written differently, and I would even say that it may make sense to introduce a variable for that expression. Additionally I don't see how the branch using inf will always lead to finite results.

Finally as indicated in #2056 I believe we should use eps as about 2.2e-16, i.e. the distance between 1 and the next floating point number, not 1.1e-16.

beutlich commented 4 years ago

@sielemann Can you help to clarify? Thanks.

sielemann commented 4 years ago

The expressions involving eps are used to distinguish structurally different cases. I recall that I defined these given the expected order of magnitude of all inputs (see plots in HTML documentation for some samples). With 100Modelica.Constants.eps, I agree that the conditions are very tight, and I would think that 1000Modelica.Constants.eps or a larger "small mass flow rate" (for Modelica.Fluid.Pipes.BaseClasses.WallFriction.Laminar.massFlowRate_dp_staticHead and the like)/"small pressure difference" (for Modelica.Fluid.Pipes.BaseClasses.WallFriction.Laminar.pressureLoss_m_flow_staticHead and the like) just as used in other correlations likely also work.

We could also introduce an new input eps with default 1000*Modelica.Constants.eps or an even higher value. Based on the order of magnitude of the arguments much higher values could make sense, too.

@HansOlsson Your rules make sense but within the function we do not know how the arguments were computed. We know that the key arguments for the eps comparison are the derivatives y0d, y1d and the slope computed from y0, y1 and (x1-x0).

Yes, the expression y1d + y0d - 2*Delta0 appears about nine times (with but signs), and it would make the code much more readable to have a variable for this.

carlj-w commented 4 years ago

Should the comparisons involve the machine constant eps at all? I am not saying it should not, but I don't know if it should.

Finally as indicated in #2056 I believe we should use eps as about 2.2e-16, i.e. the distance between 1 and the next floating point number, not 1.1e-16.

Possibly in the future, but given the current description ("Biggest number such that 1.0 + eps = 1.0") it should be 1.1e-16.

HansOlsson commented 4 years ago

Should the comparisons involve the machine constant eps at all? I am not saying it should not, but I don't know if it should.

I would assume that if we switch to super-precise machines with eps being 1e-100 we should change this criteria in some way. However, I don't see such a switch happening at the moment.

A few years ago a single precision machine with eps around 1e-8 would have made sense - now it is less common, but for such precision we need to be really careful, and in that case using 1e-13 based on current values would likely be really bad, and even 1000*eps seems odd.

Finally as indicated in #2056 I believe we should use eps as about 2.2e-16, i.e. the distance between 1 and the next floating point number, not 1.1e-16.

Possibly in the future, but given the current description ("Biggest number such that 1.0 + eps = 1.0") it should be 1.1e-16.

That why I'm arguing for also changing the descriptions in #2506.

HansOlsson commented 4 years ago

Just to double-check @carlj-w : you have verified that it is eps causing the problem, and not e.g., inf which also has a bad value?

carlj-w commented 4 years ago

Just to double-check @carlj-w : you have verified that it is eps causing the problem, and not e.g., inf which also has a bad value?

Yes, if I replace eps with 1e-15 (without replacing inf) the models work fine. If I replace inf with 1e60 (the value from an unconfigured ModelicaServices, without replacing eps), they do not work.

That said, the inf might be problematic as well, but I am not sure.

Should the comparisons involve the machine constant eps at all? I am not saying it should not, but I don't know if it should.

I would assume that if we switch to super-precise machines with eps being 1e-100 we should change this criteria in some way. However, I don't see such a switch happening at the moment.

I was thinking along the lines if the values are picked to be relevant in the application domain or actual machine precision considerations. The function as presented is quite general and the latter scenario could be the case, but I do not know.