[yt-dev] Volume Rendering API is very complicated when doing non-default things

Thu Jul 7 10:20:21 PDT 2016

Hi,

I think getting rid of it is a good plan.

I too have been frustrated and confused at times with the new API, but I
thought it was just me not being totally up on it.  I think it opens up so
many new opportunities and possibilities, and I absolutely don't want to be
misunderstood that I'm a huge proponent of the new stuff, but I think there
is a level of smoothing-out we could do to make things simpler.

A few things off hand:

 * Right now it's very difficult to get performance out of the off-axis
projections that is reasonable, because of the way the VR handles kdTree
construction and vertex-centering.  With getting rid of auto, we could fix
this.  (OAPs don't need either!)
 * The utilities.py file has a lot of boilerplate code I'm not a huge fan
of, that I think could be stuck elsewhere.
 * TransferFunctionHelper is marvelous, but maybe creates another level of
indirection we don't want.

Anyway, I think Nathan has a plan, and I'm going to also spend time this
afternoon digging in to see if I can find any obvious places with minimal
impact to improve things.

One possible thing to consider is delaying a release of 3.3 until we are
happy with the API, and in the process, undoing the feature freeze.  I'm
kind of torn on this.

-Matt

On Wed, Jul 6, 2016 at 5:09 PM, Andrew Myers <atmyers2 at gmail.com> wrote:

> What do people think about getting rid of the auto keyword altogether? It
> seems like we could postpone actually building the AMRKDTree until it is
> needed (like, when show() is called). All the properties needed to do so
> (fields, logging options) can be controlled through the VolumeSource, with
> sensible defaults.
>
>
> On Wed, Jul 6, 2016 at 2:20 PM, Nathan Goldbaum <nathan12343 at gmail.com>
> wrote:
>
>>
>>
>> On Wed, Jul 6, 2016 at 4:15 PM, Cameron Hummels <chummels at gmail.com>
>> wrote:
>>
>>> I agree that this is an issue.  I think the easiest way forward is just
>>> to provide an example or two where we don't use auto=True, and show what
>>> needs to be done in the instance where it isn't done automatically.  I
>>> don't think it necessitates an overhaul, but perhaps others disagree.
>>> Since 3.3 will be the first release with the new VR interface, I think this
>>> would be appropriate to include before the release.
>>>
>>
>> I think more documentation is part of the solution, but I also think
>> there's low-hanging fruit to improve the API.
>>
>> For example, I don't think users should ever need to manually create an
>> AMRKDTree object. I should be able to control the creation of the volume
>> from the VolumeSource object itself.
>>
>>
>>>
>>> Cameron
>>>
>>> On Wed, Jul 6, 2016 at 1:57 PM, Nathan Goldbaum <nathan12343 at gmail.com>
>>> wrote:
>>>
>>>> Hi all,
>>>>
>>>> tl;dr:
>>>>
>>>> I have a high-level issue with the volume rendering API. I think there
>>>> are some simple things that can be done to make it simpler to use, but I'm
>>>> not sure how much work it will take to get us there.
>>>>
>>>> ----------------------------------------------------
>>>>
>>>> Today I was looking at issue #1234:
>>>> https://bitbucket.org/yt_analysis/yt/issues/1234/volume-rendering-example-in-loading
>>>>
>>>> This refers to an example in our documentation that uses the old volume
>>>> rendering API to make a very pretty image. I wanted to retain that image,
>>>> which led me to try to make a fully customized volume rendering using the
>>>> new interface for the first time, and it left me very confused. If I was
>>>> completely unfamiliar with yt internals and not comfortable looking at the
>>>> yt source code, I don't think I would have been able to successfully
>>>> recreate the image using the new API.
>>>>
>>>> To make that concrete, here's how you would construct the volume
>>>> rendering using the old interface:
>>>>
>>>> https://bpaste.net/show/58194a5a1352
>>>>
>>>> And here's what I did to make the same image using the new volume
>>>> rendering interface:
>>>>
>>>> https://bpaste.net/show/9fba5872e532
>>>>
>>>> Three lines of code in the old interface expand into 26 lines of code
>>>> in the new interface. I also need to do scary-looking undocumented things
>>>> like manually creating an AMRKDTree to be able to get the field logging
>>>> setting I wanted. I also need to know about what's going on under the hood
>>>> in the VolumeSource class since I wanted to avoid setting auto=True, which
>>>> does a lot of basic setup, but also does some unnecessary computation.
>>>>
>>>> That said, some of the additional boilerplate constitutes an
>>>> improvement in clarity. For example, I like how I'm setting camera
>>>> attributes now instead of setting keyword arguments in the Camera
>>>> initializer.
>>>>
>>>> The main problem I see is that the VR interface attempts to do too much
>>>> "magic" by default. If auto=True is set when the VolumeSource is created,
>>>> an AMRKDTree is constructed, fields to render are set, whether or not the
>>>> rendering happens in log space is determined by looking at the take_log
>>>> setting in the FieldInfo, and a transfer function is created based on a
>>>> TransferFunctionHelper instance. If auto=False, it's not clear about how
>>>> one sets this stuff up without looking carefully at what happens inside the
>>>> VolumeSource code when auto=True.
>>>>
>>>> It would be nice if there were a better story in the auto=False
>>>> universe. For example, the VolumeSource object could grow a way to
>>>> *automatically* create AMRKDtree instances when needed, after a user sets
>>>> the field to render and sets which fields should be rendered in log space.
>>>> This is almost possible in the current API, but the VolumeSource object
>>>> would need to grow an API that allows the user to control the field logging
>>>> behavior - right now this is only controllable at the level of the
>>>> AMRKDTree object.
>>>>
>>>> I'm curious whether anyone has suggestions or ideas for ways forward on
>>>> this. Is this a documentation problem (i.e. do we just need to add a ton of
>>>> examples where we do customized volume renderings in a somewhat baroque but
>>>> very explicitt style) or indicative of deeper issues with the new VR
>>>> interface? Should the issues I'm raising be dealt with before we release yt
>>>> 3.3 or could we patch it afterwards?
>>>>
>>>> All that said, I don't particularly want to embark on a several
>>>> weeks-long yak-shaving expedition in the volume rendering code and I can't
>>>> volunteer to lead any kind of overhaul. I am willing to make small
>>>> usability improvements, though, if that's what we decide we should do.
>>>>
>>>> Hope that wasn't too much of a novel, I'd love to hear comments about
>>>> this.
>>>>
>>>> -Nathan
>>>>
>>>> _______________________________________________
>>>> yt-dev mailing list
>>>> yt-dev at lists.spacepope.org
>>>> http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org
>>>>
>>>>
>>>
>>>
>>> --
>>> Cameron Hummels
>>> NSF Postdoctoral Fellow
>>> Department of Astronomy
>>> California Institute of Technology
>>> http://chummels.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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.spacepope.org/pipermail/yt-dev-spacepope.org/attachments/20160707/eaca6c34/attachment.html>