#1819: CarpetMask: add option to exclude boxes -------------------------------------+-------------------------------------- Reporter: bmundim | Owner: eschnett Type: enhancement | Status: new Priority: unset | Milestone: Component: Carpet | Version: development version Keywords: CarpetMask CarpetReduce | -------------------------------------+-------------------------------------- I would like to be able to exclude boxes in addition to spherical surfaces when setting the CarpetMask::weight (which is used by CarpetReduce to exclude regions when performing reductions). The following pull request implements this functionality:
{{{ https://bitbucket.org/eschnett/carpet/pull-requests/5/bcm-carpetmask/diff }}}
I have some parameter files I used to test my implementation. I can turn them into testsuites after the revision of this ticket.
#1819: CarpetMask: add option to exclude boxes --------------------------+------------------------------------------------- Reporter: bmundim | Owner: eschnett Type: enhancement | Status: new Priority: unset | Milestone: Component: Carpet | Version: development version Resolution: | Keywords: CarpetMask CarpetReduce --------------------------+-------------------------------------------------
Comment (by eschnett):
The code looks fine; please add the test cases.
#1819: CarpetMask: add option to exclude boxes --------------------------+------------------------------------------------- Reporter: bmundim | Owner: eschnett Type: enhancement | Status: new Priority: unset | Milestone: Component: Carpet | Version: development version Resolution: | Keywords: CarpetMask CarpetReduce --------------------------+-------------------------------------------------
Comment (by bmundim):
Ok, I will add these three files as test cases and will work on another one with AMR.
#1819: CarpetMask: add option to exclude boxes --------------------------+------------------------------------------------- Reporter: bmundim | Owner: eschnett Type: enhancement | Status: review Priority: unset | Milestone: Component: Carpet | Version: development version Resolution: | Keywords: CarpetMask CarpetReduce --------------------------+------------------------------------------------- Changes (by bmundim):
* status: new => review
#1819: CarpetMask: add option to exclude boxes --------------------------+------------------------------------------------- Reporter: bmundim | Owner: eschnett Type: enhancement | Status: review Priority: unset | Milestone: Component: Carpet | Version: development version Resolution: | Keywords: CarpetMask CarpetReduce --------------------------+-------------------------------------------------
Comment (by rhaas):
Bruno, Erik: since there are now test cases, should this go in (after the release I guess)?
#1819: CarpetMask: add option to exclude boxes --------------------------+------------------------------------------------- Reporter: bmundim | Owner: eschnett Type: enhancement | Status: reviewed_ok Priority: unset | Milestone: Component: Carpet | Version: development version Resolution: | Keywords: CarpetMask CarpetReduce --------------------------+------------------------------------------------- Changes (by eschnett):
* status: review => reviewed_ok
Comment:
Yes, this is now fine.
#1819: CarpetMask: add option to exclude boxes --------------------------+------------------------------------------------- Reporter: bmundim | Owner: eschnett Type: enhancement | Status: reviewed_ok Priority: unset | Milestone: Component: Carpet | Version: development version Resolution: | Keywords: CarpetMask CarpetReduce --------------------------+-------------------------------------------------
Comment (by bmundim):
Note that I haven't run these tests in a while. Before committing, it would be a good idea rerunning these tests and/or taking a look at the data again, ie visualizing it. I am sorry I won't have time to do so before the release. Please feel free to give it a try if you have time. Thanks.
#1819: CarpetMask: add option to exclude boxes --------------------------+------------------------------------------------- Reporter: bmundim | Owner: eschnett Type: enhancement | Status: reopened Priority: unset | Milestone: Component: Carpet | Version: development version Resolution: | Keywords: CarpetMask CarpetReduce --------------------------+------------------------------------------------- Changes (by rhaas):
* status: reviewed_ok => reopened
Comment:
Bruno: unfortunately the tests fail on my laptop. Unless they are fixed the patch cannot be applied. I tried this with your branch from you bitbucket repo as well as with a branch that I rebased onto master (rhaas/carpetmask in the main repo).
I get differences like this: {{{#!diff diff -rw --from repos/carpet/CarpetMask/test repos/carpet/CarpetMask/test/test_mask_exclude_boxes/weight.x.asc TEST/sim/CarpetMask/test_mask_exclude_boxes/weight.x.asc 37c37 < 0 0 0 0 0 34 22 22 0 0.6 0 0 0 ---
0 0 0 0 0 34 22 22 0 0.6 0 0 1
84c84 < 1 0 0 0 0 34 22 22 1 0.6 0 0 0 ---
1 0 0 0 0 34 22 22 1 0.6 0 0 1
131c131 < 2 0 0 0 0 34 22 22 2 0.6 0 0 0 ---
2 0 0 0 0 34 22 22 2 0.6 0 0 1
diff -rw --from repos/carpet/CarpetMask/test repos/carpet/CarpetMask/test/test_mask_exclude_boxes_exterior/weight.x.asc TEST/sim/CarpetMask/test_mask_exclude_boxes_exterior/weight.x.asc 37c37 < 0 0 0 0 0 34 22 22 0 0.6 0 0 0 ---
0 0 0 0 0 34 22 22 0 0.6 0 0 1
84c84 < 1 0 0 0 0 34 22 22 1 0.6 0 0 0 ---
1 0 0 0 0 34 22 22 1 0.6 0 0 1
131c131 < 2 0 0 0 0 34 22 22 2 0.6 0 0 0 ---
2 0 0 0 0 34 22 22 2 0.6 0 0 1
}}}
#1819: CarpetMask: add option to exclude boxes --------------------------+------------------------------------------------- Reporter: bmundim | Owner: eschnett Type: enhancement | Status: reopened Priority: unset | Milestone: Component: Carpet | Version: development version Resolution: | Keywords: CarpetMask CarpetReduce --------------------------+------------------------------------------------- Description changed by rhaas:
Old description:
I would like to be able to exclude boxes in addition to spherical surfaces when setting the CarpetMask::weight (which is used by CarpetReduce to exclude regions when performing reductions). The following pull request implements this functionality:
{{{ https://bitbucket.org/eschnett/carpet/pull-requests/5/bcm-carpetmask/diff }}}
I have some parameter files I used to test my implementation. I can turn them into testsuites after the revision of this ticket.
New description:
I would like to be able to exclude boxes in addition to spherical surfaces when setting the CarpetMask::weight (which is used by CarpetReduce to exclude regions when performing reductions). The following pull request implements this functionality:
https://bitbucket.org/eschnett/carpet/pull-requests/5/bcm-carpetmask/diff
I have some parameter files I used to test my implementation. I can turn them into testsuites after the revision of this ticket.
--
trac@lists.einsteintoolkit.org
-
Einstein Toolkit