Add tolerance to box in compute_point_locations_try_all #15102
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bergbauer
reviewed
Apr 14, 2023
source/grid/grid_tools.cc
Outdated
| const auto &box = leaf.first; | ||
| const double relative_tolerance = 1e-12; | ||
| const BoundingBox<spacedim> box = leaf.first.create_extended( | ||
| relative_tolerance * leaf.first.side_length(0)); |
There was a problem hiding this comment.
I think it would be better if we scale each side length with a relative tolerance. Thanks for tackling this! @kronbichler
There was a problem hiding this comment.
What about creating BB::create_extended_relative()? @bergbauer could write this ;)
There was a problem hiding this comment.
I added @bergbauer's suggestion (via his own commit) to this PR.
peterrum
reviewed
Apr 14, 2023
source/grid/grid_tools.cc
Outdated
| const auto &box = leaf.first; | ||
| const double relative_tolerance = 1e-12; | ||
| const BoundingBox<spacedim> box = leaf.first.create_extended( | ||
| relative_tolerance * leaf.first.side_length(0)); |
There was a problem hiding this comment.
What about creating BB::create_extended_relative()? @bergbauer could write this ;)
kronbichler
force-pushed
the
add_tolerance_compute_point_locations
branch
from
e2c4bf3
to
cce10ff
Compare
peterrum
approved these changes
Apr 14, 2023
|
Can we have another review on this? #15035 depends on this to fix the failing tests there. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In #15035, we observed a test failure of
numerics/generalized_interpolation_02because of the following assertion:dealii/include/deal.II/numerics/fe_field_function.templates.h
Lines 543 to 545 in f5f72b0
What is happening there is that a point ends up being outside the triangulation by roundoff (1e-16), which can happen during a typical program run. Looking into the code, I realize the following condition
dealii/source/grid/grid_tools.cc
Lines 5835 to 5837 in f5f72b0
intersects(box)without tolerance. Since I don't know how to teachboosta tolerance (maybe you do @luca-heltai?), I think the right way is to make the bounding box we accept larger by a roundoff margin, say1e-12relative tolerance. I added a test that checks this for the actual numbers where it happened (outside by1e-16), by outside of1e-8, and outside a lot. I think this way the code makes more sense.