-
Notifications
You must be signed in to change notification settings - Fork 708
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
Better document what tolerance to use in MappingCartesian. #14103
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -55,19 +55,28 @@ is_cartesian(const CellType &cell) | |
if (!cell->reference_cell().is_hyper_cube()) | ||
return false; | ||
|
||
const double tolerance = 1e-14; | ||
const double abs_tol = 1e-15; | ||
const double rel_tol = 1e-14; | ||
const auto bounding_box = cell->bounding_box(); | ||
const auto & bounding_vertices = bounding_box.get_boundary_points(); | ||
const auto cell_measure = | ||
const auto bb_diagonal_length_squared = | ||
bounding_vertices.first.distance_square(bounding_vertices.second); | ||
|
||
for (const unsigned int v : cell->vertex_indices()) | ||
{ | ||
const double vertex_tolerance = | ||
tolerance * std::max(cell->vertex(v).norm_square(), cell_measure); | ||
|
||
if (cell->vertex(v).distance_square(bounding_box.vertex(v)) > | ||
vertex_tolerance) | ||
// Choose a tolerance that takes into account both that vertices far | ||
// away from the origin have only a finite number of digits | ||
// that are considered correct (an "absolute tolerance"), as well as that | ||
// vertices are supposed to be close to the corresponding vertices of the | ||
// bounding box (a tolerance that is "relative" to the size of the cell). | ||
// | ||
// We need to do it this way because when a vertex is far away from | ||
// the origin, computing the difference between two vertices is subject | ||
// to cancellation. | ||
const double tolerance = std::max(abs_tol * cell->vertex(v).norm_square(), | ||
rel_tol * bb_diagonal_length_squared); | ||
Comment on lines
+76
to
+77
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is not your fault but already present in @gassmoeller's fix: I'm wondering whether this is the correct magnitude for the tolerances. Shouldn't we use something like |
||
|
||
if (cell->vertex(v).distance_square(bounding_box.vertex(v)) > tolerance) | ||
return false; | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you mind adding the term "cancellation" here, as this is exactly what happens: Far away from the origin, the difference between two vertices is subject to cancellation according to this quantity.