cell padding in RegionSelector
Hi all, Matt and I are finishing up our changes to the selector functions and ran into a feature which may or may not be currently used. In RegionSelector, there is a parameter _dx_pad to allow for cells just outside the selected region to be included. Is anyone using that feature or can it be deprecated? Douglas Rudd Scientific Computing Consultant Research Computing Center drudd@uchicago.edu
Hi again,
A bit more information that people might find useful interpreting my previous question (sorry for the verbosity).
I added a check in the RegionSelector to ensure that a region larger than the domain was not requested (which may
be a sign of incorrect logic in the request). It turns out that some parts of the code automatically request more than the
domain, and I attributed that to dx_pad.
dx_pad is set to 0.5 in YTRegionBase, and not touched anywhere else in the code that I could find. Setting it to 0 continues
to pass all tests (its behavior does not appear to be used).
That didn't eliminate requests for regions larger than the domain, however, due to the use of the grid_patch function, which
attempts to extend the region but doesn't check if the region already contained the new volume (due to periodicity). I don't
want to make any changes here, though.
Here is my proposed solution that I'd like to make sure won't break other people's codes not reflected in the tests:
1) Remove _dx_pad from YTRegionBase and from RegionSelector. Tests will function as normal, the only risk is for user
code that expects the padding.
2) Change my test in RegionSelector to accept region requests that are larger than the domain, provided the domain is
periodic in that dimension. If not, it will still generate an error, because a ghost zone was requested that does not exist.
Thoughts?
Douglas Rudd
Scientific Computing Consultant
Research Computing Center
drudd@uchicago.edu
On Aug 15, 2013, at 10:11 AM, Douglas Harvey Rudd
Hi all,
Matt and I are finishing up our changes to the selector functions and ran into a feature which may or may not be currently used.
In RegionSelector, there is a parameter _dx_pad to allow for cells just outside the selected region to be included. Is anyone using that feature or can it be deprecated?
Douglas Rudd Scientific Computing Consultant Research Computing Center drudd@uchicago.edu
_______________________________________________ yt-dev mailing list yt-dev@lists.spacepope.org http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org
Hi Doug,
Thanks for writing. I believe that the _dx_pad comes from a
discussion Britton and Stephen had a few years ago about how to handle
boxes that were partially included in halo finding operations, or
possibly projections of thin regions. This comes back to the usage of
RegionStrict and Region, where one would select cells within a given
region *exclusively* and another would allow partial cell selection.
My own preference is to remove the padding, but I understand there may
be valid reasons to retain it. I do not believe that user-facing code
is likely to use these two different regions; I think that they will
only be used independently by internal yt code, which would be exposed
through things like projections and so on. So changing it may change
behavior as the user sees, but I do not think that it is likely that
users *themselves* would rely on this directly.
My preference is #1, but I'd like to hear from at least Britton to get
some context on whether or not this splitting of region behavior is
still necessary.
-Matt
On Thu, Aug 15, 2013 at 1:10 PM, Douglas Harvey Rudd
Hi again,
A bit more information that people might find useful interpreting my previous question (sorry for the verbosity).
I added a check in the RegionSelector to ensure that a region larger than the domain was not requested (which may be a sign of incorrect logic in the request). It turns out that some parts of the code automatically request more than the domain, and I attributed that to dx_pad.
dx_pad is set to 0.5 in YTRegionBase, and not touched anywhere else in the code that I could find. Setting it to 0 continues to pass all tests (its behavior does not appear to be used).
That didn't eliminate requests for regions larger than the domain, however, due to the use of the grid_patch function, which attempts to extend the region but doesn't check if the region already contained the new volume (due to periodicity). I don't want to make any changes here, though.
Here is my proposed solution that I'd like to make sure won't break other people's codes not reflected in the tests:
1) Remove _dx_pad from YTRegionBase and from RegionSelector. Tests will function as normal, the only risk is for user code that expects the padding.
2) Change my test in RegionSelector to accept region requests that are larger than the domain, provided the domain is periodic in that dimension. If not, it will still generate an error, because a ghost zone was requested that does not exist.
Thoughts?
Douglas Rudd Scientific Computing Consultant Research Computing Center drudd@uchicago.edu
On Aug 15, 2013, at 10:11 AM, Douglas Harvey Rudd
wrote: Hi all,
Matt and I are finishing up our changes to the selector functions and ran into a feature which may or may not be currently used.
In RegionSelector, there is a parameter _dx_pad to allow for cells just outside the selected region to be included. Is anyone using that feature or can it be deprecated?
Douglas Rudd Scientific Computing Consultant Research Computing Center drudd@uchicago.edu
_______________________________________________ yt-dev mailing list yt-dev@lists.spacepope.org http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org
_______________________________________________ yt-dev mailing list yt-dev@lists.spacepope.org http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org
Hi all,
I wanted to quickly write back just to note that in the next day or
two, I will be issuing a pull request to replace all calls to
region_strict in yt-3.0 to calls to just region. This reflects that
the two objects are now the same, and that we always provide *no* dx
padding. (And, the region_strict object doesn't even exist anymore
and results in an error!)
Those of you who have built on top of region() objects may see minor
changes in behavior; I think we should address those as they crop up,
and determine how to provide a consistent experience. The places I
see this potentially causing problems are in projections that utilize
data regions as their sources; I am starting to think that for
situations (like thin projections) that rely on projecting through
varying cell-size regions, we may be better suited to using
non-interpolating off-axis projections, as those will correctly
terminate rays *inside* cells.
-Matt
On Thu, Aug 15, 2013 at 3:14 PM, Matthew Turk
Hi Doug,
Thanks for writing. I believe that the _dx_pad comes from a discussion Britton and Stephen had a few years ago about how to handle boxes that were partially included in halo finding operations, or possibly projections of thin regions. This comes back to the usage of RegionStrict and Region, where one would select cells within a given region *exclusively* and another would allow partial cell selection.
My own preference is to remove the padding, but I understand there may be valid reasons to retain it. I do not believe that user-facing code is likely to use these two different regions; I think that they will only be used independently by internal yt code, which would be exposed through things like projections and so on. So changing it may change behavior as the user sees, but I do not think that it is likely that users *themselves* would rely on this directly.
My preference is #1, but I'd like to hear from at least Britton to get some context on whether or not this splitting of region behavior is still necessary.
-Matt
On Thu, Aug 15, 2013 at 1:10 PM, Douglas Harvey Rudd
wrote: Hi again,
A bit more information that people might find useful interpreting my previous question (sorry for the verbosity).
I added a check in the RegionSelector to ensure that a region larger than the domain was not requested (which may be a sign of incorrect logic in the request). It turns out that some parts of the code automatically request more than the domain, and I attributed that to dx_pad.
dx_pad is set to 0.5 in YTRegionBase, and not touched anywhere else in the code that I could find. Setting it to 0 continues to pass all tests (its behavior does not appear to be used).
That didn't eliminate requests for regions larger than the domain, however, due to the use of the grid_patch function, which attempts to extend the region but doesn't check if the region already contained the new volume (due to periodicity). I don't want to make any changes here, though.
Here is my proposed solution that I'd like to make sure won't break other people's codes not reflected in the tests:
1) Remove _dx_pad from YTRegionBase and from RegionSelector. Tests will function as normal, the only risk is for user code that expects the padding.
2) Change my test in RegionSelector to accept region requests that are larger than the domain, provided the domain is periodic in that dimension. If not, it will still generate an error, because a ghost zone was requested that does not exist.
Thoughts?
Douglas Rudd Scientific Computing Consultant Research Computing Center drudd@uchicago.edu
On Aug 15, 2013, at 10:11 AM, Douglas Harvey Rudd
wrote: Hi all,
Matt and I are finishing up our changes to the selector functions and ran into a feature which may or may not be currently used.
In RegionSelector, there is a parameter _dx_pad to allow for cells just outside the selected region to be included. Is anyone using that feature or can it be deprecated?
Douglas Rudd Scientific Computing Consultant Research Computing Center drudd@uchicago.edu
_______________________________________________ yt-dev mailing list yt-dev@lists.spacepope.org http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org
_______________________________________________ yt-dev mailing list yt-dev@lists.spacepope.org http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org
participants (2)
-
Douglas Harvey Rudd
-
Matthew Turk