- changed status to open
- removed comment
suspicious code for operator+ in bboxset2
Operator+ in bboxset2 seems to actually implement operator^ but I don't quite understand what they are all doing so, please review.
When running with CARPET_DEBUG and CARPET_USE_BBOXSET2 the code dies with an assert due to a non-empty box intersection from inside CARPET_DEBUG code which seems to be due to the (invalid) assumption that multiple box.exterior would not overlap.
Finally the last patch fixes an possible obscure issue where CarpetIOHDF5 will ignore the checkpoint=no tag of a grid variable if the checkpoint file does indeed contain such a variable (since eg it was written with a code version that still had checkpoint=yes).
Keyword: Carpet
Comments (8)
-
-
- removed comment
Patch 0001 seems fine. I still have to digest the others.
-
- changed status to open
- removed comment
Where is the
+
operator that combines different box.exteriors? Is this the one changed in patch 0003?Operator
^
calculates the symmetric set difference. Due to the internal representation used by bboxset2, which is based on symmetric set differences, this operator is especially efficient. I believe the current code is correct, and the patch only makes things less efficient. Please do not apply it.Patch 0003 looks correct. I do not know why the fine grid boundaries should be disjoint. Please apply it.
-
reporter - changed status to resolved
- removed comment
Applied as has dcd0afee1d6ce0572a789be017ccdd5b0d1824d3 and 60183e2b8852ad6f6261ac6cdb0d6ffc9cd4dcb4 of Carpet.
I have left out 0002 as requested. I added instead a comment to operator+ explaining that yes indeed this is correct and the symmetric difference of disjoint sets is the union of the sets as commit 0d17588fccdb2e4882d3e0edb4ce0e29544c20d8.
-
reporter - removed comment
Replying to [comment:3 eschnett]:
Where is the
+
operator that combines different box.exteriors? Is this the one changed in patch 0003? yes, it is in patch 0003. -
- removed comment
A correction, for the record -- I misspoke slightly.
-
- changed status to open
- removed comment
Roland, I do not see your commit on the master branch.
-
reporter - changed status to resolved
- removed comment
Sorry, I had forgotten to git push after running the tests. Done now.
- Log in to comment