Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
carlj-w opened this issue Sep 21, 2020 · 7 comments
Open

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

carlj-w opened this issue Sep 21, 2020 · 7 comments
Labels
L: Fluid Issue addresses Modelica.Fluid (excl. Dissipation)

Comments

@carlj-w
Copy link
Contributor

carlj-w commented Sep 21, 2020

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:

  • ModelicaTest.Fluid.TestComponents.Vessels.TestMixingVolumesPressureStates
  • ModelicaTest.Fluid.TestPipesAndValves.DynamicPipeClosingValve

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
Copy link
Contributor

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

  • The difference is squared (or cubed) and thus we shouldn't compare it with "eps" but with sqrt(eps) or something like that.
  • We use x/y; and compare x with eps. We should instead compare x/y with eps, or x with eps*y.
    Thus we need to analyze it in more detail, and not just randomly change it.

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
Copy link
Member

@sielemann Can you help to clarify? Thanks.

@beutlich beutlich added the L: Fluid Issue addresses Modelica.Fluid (excl. Dissipation) label Sep 21, 2020
@sielemann
Copy link

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
Copy link
Contributor Author

carlj-w commented Sep 22, 2020

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor Author

carlj-w commented Sep 22, 2020

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
L: Fluid Issue addresses Modelica.Fluid (excl. Dissipation)
Projects
None yet
Development

No branches or pull requests

4 participants