- changed status to open
- removed comment
Very large grids with bboxset2
Issue #1846
closed
Using the ET_2015_11 release, Carpet incorrectly replaces sets of components with their containing bbox, leading to a large increase in the number of grid points on the corresponding refinement level. This reduces simulation speed and causes increased memory usage (a lot). Using the same parameter file, this problem is visible with ET_2015_11, but not with ET_2014_05. Setting CARPET_DISABLE_BBOXSET2, or setting CarpetRegrid2::min_fraction = 1, both work around the problem. The bboxset2 code was enabled between these two releases, so this points to a problem with the code in bboxset2 that is used to determine whether to use the containing box or not. I attach parameter files and grid structure visualisations which demonstrate the problem. The parameters files differ only in the use of min_fraction = 1, but the same change is observed if you compile with -DCARPET_DISABLE_BBOXSET2 in CPPFLAGS.
Keyword: backport
Comments (13)
-
reporter
-
- removed comment
Can you check whether CARPET_AVOID_LAMBDA is set on your system?
-
- removed comment
This patch
diff --git a/CarpetLib/src/bboxset2.hh b/CarpetLib/src/bboxset2.hh index f0d7a9d..2d58925 100644 --- a/CarpetLib/src/bboxset2.hh +++ b/CarpetLib/src/bboxset2.hh @@ -943,7 +943,7 @@ template <typename T, int D> size_type bboxset<T, D>::size() const { #else TRAVERSE_SUBSETS1(const T &pos(_1); const bboxset1 &subset(_2); { const size_type subset_size = subset.size(); - total_size += (pos - old_pos) * old_subset_size; + total_size += old_subset_size == 0 ? 0 : (pos - old_pos) * old_subset_size; old_pos = pos; old_subset_size = subset_size; });may prevent an integer overflow, if CARPET_AVOID_LAMBDA is set.
-
- removed comment
Looking at the code trying to show it to Tim Dietrich, I rather suspect the issue may be
T old_pos = numeric_limits<T>::min()ins bboxset2.hh which for a signed T is very large negative number, hence pos-old_pos is very large and positive. -
- removed comment
Yes, that is the problem. This will occur during the first iteration. During this iteration, old_subset_size==0, so the patch above corrects the problem.
-
reporter
- removed comment
Replying to [comment:2 eschnett]:
Can you check whether CARPET_AVOID_LAMBDA is set on your system?
As far as I can tell, nothing should be setting it. It is not set in the optionlist (datura.cfg), and it doesn't seem that anything in Carpet defines it. Thank you for the patch. It does not apply to the release version, probably because the Carpet source code was reformatted. I applied the change introduced by the patch manually and reran the test. Unfortunately, the grid structure is still bad, which is consistent with CARPET_AVOID_LAMBDA not being set, since the patch only refers to the part of the code which is run when CARPET_AVOID_LAMBDA is defined.
-
- removed comment
The case !CARPET_AVOID_LAMBDA was already correct, I only ported the correction.
Can you add a statement into the code checking whether CARPET_AVOID_LAMBDA is set?
-
- removed comment
Found the problem; bboxset2::size calculated the volume of the set instead of the number of points. Corrected on the master.
This should be backported once it has been confirmed to work.
-
- removed comment
I have applied the patch to the release branch and can confirm that it fixes the problem in our original test case.
-
- removed comment
-
- changed status to resolved
- removed comment
The fix has been backported to the release branch.
-
- removed comment
Barry: thank you. Frank: should this be announced on the mailing list:
-
- edited description
- changed status to closed
- Log in to comment
- Assignee
- Type
- Priority
- Status
- closed
- Component
- Carpet
- Milestone
- –
- Version
- development version
- Votes
- 0
- Watchers
- 0
