[yt-dev] Sequence of buff_size

Matthew Turk matthewturk at gmail.com
Fri Apr 1 12:24:09 PDT 2016


Hi Yi-Hao,

OK -- I agree, this is behavior we should fix.  Let's try to minimize
impact disruption if we can, but it ought to be fixed.  Thanks for
digging through and finding this!

On Fri, Apr 1, 2016 at 1:44 PM, Yi-Hao Chen <ychen at astro.wisc.edu> wrote:
>
> On Fri, Apr 1, 2016 at 11:23 AM, Matthew Turk <matthewturk at gmail.com> wrote:
>>
>> On Fri, Apr 1, 2016 at 11:21 AM, Nathan Goldbaum <nathan12343 at gmail.com>
>> wrote:
>> >
>> >
>> > On Fri, Apr 1, 2016 at 11:11 AM, Matthew Turk <matthewturk at gmail.com>
>> > wrote:
>> >>
>> >> Hi Yi-Hao,
>> >>
>> >> The actual usage of _MPL.c shouldn't be user-facing.  So I think as
>> >> long as the things that are public APIs (such as the data returned
>> >> from a FixedResolutionBuffer object) remain the same, I am a strong +1
>> >> on this.
>> >
>> >
>> > The trouble is, if we want to make the user interface match what people
>> > expect, i.e. buff_size is (nx, ny), where nx it the number of pixels
>> > along x
>> > and ny is the number of pixels along y, we'll need to change the
>> > behavior
>> > here.
>>
>> Hmmm .. but we transpose in most places, like
>> yt/geometry/coordinates/cartesian_coordinate_handler.py .
>
>
> I think the transpose of the image array is to fit the behavior of imshow(),
> which needs an array with the shape (ny, nx) to display nx pixels along x
> axis and ny pixels along y axis.
>
> I assumed generally people expect buff_size is (nx, ny). But if they really
> use non-square buff_size, they probably should have figured it out by trial
> and error and used (ny, nx) instead. The proposed fix will change this
> behavior.
>
>>
>> >
>> >>
>> >>
>> >> -Matt
>> >>
>> >> On Fri, Apr 1, 2016 at 10:19 AM, Yi-Hao Chen <ychen at astro.wisc.edu>
>> >> wrote:
>> >> > Hi all,
>> >> >
>> >> > Running this test script
>> >> > http://paste.yt-project.org/show/6379/
>> >> > will result in this image
>> >> > https://slack-files.com/T042F73QM-F0X0YJE3V-4a579d9431
>> >> >
>> >> > Apparently, the sequence of buff_size is switched so that
>> >> >
>> >> > set_buff_size((8,4))
>> >> >
>> >> > gives a 4 by 8 image.
>> >> >
>> >> > I have traced down the codes and found the following lines are the
>> >> > cause
>> >> > of
>> >> > the problem.
>> >> >
>> >> >
>> >> > https://bitbucket.org/yt_analysis/yt/src/d18f33211199f71e2fac4d927307b015d513a328/yt/utilities/lib/pixelization_routines.pyx?at=yt&fileviewer=file-view-default#pixelization_routines.pyx-61
>> >> >
>> >> >
>> >> >
>> >> > https://bitbucket.org/yt_analysis/yt/src/d18f33211199f71e2fac4d927307b015d513a328/yt/utilities/lib/pixelization_routines.pyx?at=yt&fileviewer=file-view-default#pixelization_routines.pyx-216
>> >> >
>> >> > If rows=nx and cols=ny, the two functions (pixelize_cartesian and
>> >> > pixelize_off_axis_cartesian) should take rows first and then cols.
>> >> >
>> >> > The same API has been around in _MPL.c since 2010 and thus changing
>> >> > it
>> >> > will
>> >> > be backward incompatible.
>> >> >
>> >> > However, to be consistent with other parts of the code (e.g. here,
>> >> > here,
>> >> > here, and here), in which buff_size = (nx, ny) is assumed and used, I
>> >> > propose to change the behavior of the above two functions.
>> >
>> >
>> > I'm +0.5 on this, but am nervous that it will change the results of user
>> > scripts. I'd like the API to match naive expectations though...
>> >
>> >>
>> >> >
>> >> > What do you think? Anyone has other suggestions?
>> >
>> >
>> > The other alternative is to document the current (annoyingly backwards)
>> > behavior.
>
> If we want to keep the current behavior and document it, many parts of the
> code will need to be changed as well.
>
>>
>> >
>> >>
>> >> >
>> >> > Thanks!
>> >> > Yi-Hao
>> >
>> >
>> > Thank you for your detective work on this, Yi-Hao!
>> >
>> >>
>> >> >
>> >> > _______________________________________________
>> >> > 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
>> >
>> >
>> >
>> > _______________________________________________
>> > 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
>
>
>
> _______________________________________________
> yt-dev mailing list
> yt-dev at lists.spacepope.org
> http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org
>



More information about the yt-dev mailing list