[yt-dev] cell padding in RegionSelector

[Matthew Turk]

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 <matthewturk at gmail.com> wrote:
> 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 <drudd at uchicago.edu> 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 at uchicago.edu
>>
>>
>>
>> On Aug 15, 2013, at 10:11 AM, Douglas Harvey Rudd <drudd at uchicago.edu>
>>  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 at uchicago.edu
>>>
>>>
>>>
>>> _______________________________________________
>>> yt-dev mailing list
>>> yt-dev at lists.spacepope.org
>>> http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org
>>
>> _______________________________________________
>> yt-dev mailing list
>> yt-dev at lists.spacepope.org
>> http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org