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

Reduce machine epsilon and add a distance epsilon #560

Merged
merged 1 commit into from
Mar 24, 2021
Merged

Conversation

ftessier
Copy link
Member

@ftessier ftessier commented Nov 22, 2019

Reduce the value of the egs++ floating-point epsilon from 1e-10 to
a value that is 4 times (2 bits away) from the true machine epsilon, and
appropriately for single and double precision versions of the code.

Add a distanceEpsilon constant that is independent of the machine
epsilon, and much larger, but still entirely sufficient for physical
dimensions (well below atomic dimensions in double precision). By
default, the base geometry sets its boundaryTolerance to this
constant. Its value is close to the previous value of epsilon (1e-10),
which should curb any change in floating point error triggers in the
geometry library at this point.

Earlier in #177, the epsilon constant was introduced to remove
hard-coded floating point comparison guards. The 1e-10 value was chosen
because it matched most of the hard-coded values in the geometry
library. However, this epsilon is now starting to be used as a general
float guard (see #525 for example), where 1e-10 is not appropriate, as
it effectively reduces the floating-point precision of the computation.
Hence the need for independent epsilon values for computations and for
geometrical purposes.

@ftessier
Copy link
Member Author

ftessier commented Nov 22, 2019

This requires a review of uses of epsilon everywhere, and boundaryTolerance as well (to a lesser extent because effectively its value has only changed from 1e-10 to about 2.2e-10).

Copy link
Contributor

@mainegra mainegra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ftessier I have tested this and I get the warning:
warning: left shift count >= width of type [-Wshift-count-overflow]
This happens in bit-wise shift expressions when the number being shifted (1) is recognized as an int which is only 32 bits. If I change 1<<50 to 1ULL<<50 the warning disappears. It is probably compiler related since in principle there shouldn't be a warning. I am using here at home gcc 7.5.0.

@ftessier
Copy link
Member Author

Oh wow, good catch, I don't think there is any harm in using 1ULL<<50?

Copy link
Collaborator

@rtownson rtownson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @mainegra's suggestion to use 1ULL.

@ftessier
Copy link
Member Author

Rebased on develop and added fixed 1ULL constant in bit shift operation.

Reduce the value of the egs++ floating-point epsilon from 10e-10 to
a value that is 4 times (2 bits away) from the true machine epsilon, and
appropriately for single and double precision versions of the code.

Add a distanceEpsilon constant that is independent of the machine
epsilon, and much larger, but still entirely sufficient for physical
dimensions (well below atomic dimensions in double precision). By
default, the base geometry sets its boundaryTolerance to this
constant. Its value is close to the previous value of epsilon (1e-10),
which should curb any change in floating point error triggers in the
geometry library at this point.

Earlier in #177, the epsilon constant was introduced to remove
hard-coded floating point comparison guards. The 1e-10 value was chosen
because it matched most of the hard-coded values in the geometry
library. However, this epsilon is now starting to be used as a general
float guard (see #525 for example), where 1e-10 is not appropriate, as
it effectively reduces the floating-point precision of the computation.
Hence the need for independent epsilon values for computations and for
geometrical purposes.
@ftessier
Copy link
Member Author

Rebased on develop.

@ftessier ftessier self-assigned this Mar 24, 2021
@ftessier ftessier merged commit fd008c2 into develop Mar 24, 2021
@ftessier ftessier deleted the fix-epsilon branch March 24, 2021 21:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants