[yt-dev] Change to defaults of ProjectionPlot and OffAxisProjectionPlot

Matthew Turk matthewturk at gmail.com
Thu Jul 17 07:10:55 PDT 2014


Hi JEff,

On Thu, Jul 17, 2014 at 8:52 AM, j s oishi <jsoishi at gmail.com> wrote:
> Hi Matt,
>
> I am struggling to understand your objection.

Well, maybe it's ill-formed.

> The unit is already changed in
> projection. Cameron's point is that the variable name is not changed, and
> without a qualifier such as integrated or projected, it does look like the
> user made a typo. Why would we want to put the burden on the user to correct
> such a common occurrence?  I guess I don't understand your objection. Could
> you expand on why you are so skittish about changing the colorbar label? I
> am assuming there is a technical objection I don't know about.

I'm definitely not opposed to making it clearer what is going on --
but the unit is changed self-consistently via projecting, because the
code passes along the units that enter into each array, multiplies
them self-consistently, and then the resultant buffer is what gets
displayed.  So it's not so much that the code is deciding to change
the unit consciously, it's that it's getting changed during the course
of processing.

It's actually not a technical objection -- which perhaps indicates
that it shouldn't be an objection at all in this case.  In the
interests of empathy for the user, it may be an improvement, so
perhaps it should happen.  But, if we do that, I'd like to cover all
the relevant bases.  There are a couple ways to make an image plot in
yt:

 * Infinitesimal slice
 * Line integral ("projection")
 * Line integral of quantity times weight, divided by line integral of weight
 * Line integral, selecting "maximum" value
 * All of above, with data source (i.e., not through whole domain)

I think if we could encode these, I would be less skittish.

>
> Also, I think adding a title is not a good solution, since it creates a
> large distance between between "projection" and "density" (for example), but
> leaves no distance between "density" and "g/cm^2".

Fair point.

>
> Furthermore, regarding Sam's objection regarding SZ and other unusual
> fields, I wonder if we should put the burden of manually entering the label
> on the least likely use case. I think, though I could be wrong, that the
> majority of Astro users are interested in density projections, in which case
> we should use "projection" by default. If we are striving to generalize
> beyond that, then I think "integrated" is the proper word.
>
> J
>
> Hi Cameron,
>
> I've done a lot of thinking about this, and I'm of two minds.
>
> 1) I do not think we should change the colorbar; we're displaying
> units there, and we should either let the person making the plot
> change the title of those units, or we should display what they are.
> 2) We should allow the plots to indicate somehow what they are, rather
> than delegating that exclusively to the filename.
>
> Do you think it would suffice to add a title to the plots?  That would
> not touch the "units" (which I am really skittish about modifying) but
> will still display the semantic information about the plot.
>
> -Matt
>
> On Tue, Jul 15, 2014 at 7:56 PM, Nathan Goldbaum <nathan12343 at gmail.com>
> wrote:
>>
>>
>>
>> On Tue, Jul 15, 2014 at 5:47 PM, Cameron Hummels <chummels at gmail.com>
>> wrote:
>>
>>>
>>> after all, we have projected_units attached to the fields.
>>
>>
>> Nope, not after unitrefactor was merged in.
>>
>>>
>>>
>>>
>>> On Tue, Jul 15, 2014 at 5:45 PM, John ZuHone <jzuhone at gmail.com> wrote:
>>>>
>>>> This is probably a bad idea, but could we add a "projected_name" keyword
>>>> (or something similar) to the add_field function, which could be
>>>> "Projected"
>>>> as the default but could be left as "" for things like SZY? It would
>>>> only be
>>>> applied if there isn't a weight field.
>>>>
>>>> I suspect folks feel that add_field already has too many keyword
>>>> parameters, but I just wanted to throw it out there.
>>>>
>>>> Sent from John ZuHone's iPad
>>>>
>>>> On Jul 15, 2014, at 8:39 PM, Cameron Hummels <chummels at gmail.com> wrote:
>>>>
>>>> Sam,
>>>>
>>>> What would you think if we changed the "Projected" prepend to
>>>> "Integrated"?  That would still apply well for "Integrated Density" as
>>>> well
>>>> as "Integrated SZY", right?
>>>>
>>>> I'm just very much against the default of having "Density (g/cm^2)" show
>>>> up on projection plots (using the 'integrated' type), because this is
>>>> misleading and it just makes it look like you made a mistake when your
>>>> units
>>>> don't match your field.  As Matt suggests, we could have the title set
>>>> to
>>>> "Projection" for projections by default which is better than the current
>>>> settings IMO, but it seems less clean than changing the colorbar label.
>>>>
>>>> Anyone else have any thoughts about this?
>>>>
>>>> Cameron
>>>> On Tue, Jul 15, 2014 at 3:50 PM, Sam Skillman <samskillman at gmail.com>
>>>> wrote:
>>>>>
>>>>> Hi Cameron,hing
>>>>>
>>>>> While it would be nice if there was a simple default that would work
>>>>> for
>>>>> all types of projections and fields, I think I'm also a -0 on this
>>>>> because
>>>>> of weird fields like the SZY, where it only makes sense as an
>>>>> integrated
>>>>> field, and Projected SZY isn't a term that is used.  I think that
>>>>> simply
>>>>> allowing others to modify the colorbar name is the more sustainable way
>>>>> in
>>>>> terms of handling all of the options for integration type as well.  I
>>>>> could
>>>>> be convinced otherwise, but I think having the units for things like
>>>>> density
>>>>> show up as g/cm^2 vs g/cm^3 should be enough for the time being.
>>>>>
>>>>> Sam
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> On Tue, Jul 15, 2014 at 7:22 AM, Matthew Turk <matthewturk at gmail.com>
>>>>> wrote:
>>>>>>
>>>>>> Hi Cameron,
>>>>>>
>>>>>> I'm -0 on this, but mainly because I don't really like changing it to
>>>>>> have that information as part of the colorbar, rather than the title
>>>>>> for instance.
>>>>>>
>>>>>> -Matt
>>>>>>
>>>>>> On Tue, Jul 15, 2014 at 12:06 AM, Cameron Hummels <chummels at gmail.com>
>>>>>> wrote:
>>>>>> > Oh, I almost forgot to show examples:
>>>>>> >
>>>>>> > current behavior of a non-weighted density projection:
>>>>>> > http://i.imgur.com/vBSRRLq.png
>>>>>> >
>>>>>> > proposed behavior of a non-weighted density projection:
>>>>>> > http://i.imgur.com/UP6f5Nh.png
>>>>>> >
>>>>>> > although i like the idea that Nathan has about having "column
>>>>>> > density" for
>>>>>> > projected density plots.
>>>>>> >
>>>>>> >
>>>>>> > On Mon, Jul 14, 2014 at 9:41 PM, Nathan Goldbaum
>>>>>> > <nathan12343 at gmail.com>
>>>>>> > wrote:
>>>>>> >>
>>>>>> >>
>>>>>> >>
>>>>>> >>
>>>>>> >> On Mon, Jul 14, 2014 at 9:35 PM, Cameron Hummels
>>>>>> >> <chummels at gmail.com>
>>>>>> >> wrote:
>>>>>> >>>
>>>>>> >>> Hey everyone,
>>>>>> >>>
>>>>>> >>> I've created a pull request which changes the defaults of the
>>>>>> >>> ProjectionPlot and OffAxisProjectionPlot, although I'm looking for
>>>>>> >>> feedback
>>>>>> >>> from the community.
>>>>>> >>>
>>>>>> >>> Right now, when you create a projection, say for "Density", it
>>>>>> >>> labels the
>>>>>> >>> colorbar with "Density" and then gives its projected units
>>>>>> >>> (instead
>>>>>> >>> of
>>>>>> >>> g/cm^3, it gives g/cm^2).  My PR is simply to change the default
>>>>>> >>> label to be
>>>>>> >>> "Projected <field>" in this case "Projected Density (g/cm^2)".
>>>>>> >>
>>>>>> >>
>>>>>> >> +1. I also sort of like the idea of special-casing  - in particular
>>>>>> >> for
>>>>>> >> density, which I think should show up as "Column Density".
>>>>>> >>
>>>>>> >>>
>>>>>> >>> It will do this in the case of non-weighted projections.
>>>>>> >>
>>>>>> >>
>>>>>> >> Also only when proj_stype = "integrate".
>>>>>> >>
>>>>>> >>>
>>>>>> >>> I think this is the expected behavior and more accurate than the
>>>>>> >>> former
>>>>>> >>> behavior, but I'm open to discussion from the rest of the dev
>>>>>> >>> community.
>>>>>> >>>
>>>>>> >>> In addition, it might be worthwhile to change the defaults on
>>>>>> >>> weighted-projections (e.g. density-weighted temperature
>>>>>> >>> projection), to give
>>>>>> >>> it and appropriate label as well, but I'm less convinced of this
>>>>>> >>> change.
>>>>>> >>> Perhaps something like "<weight_field>-Weighted <field> (units)" ?
>>>>>> >>
>>>>>> >>
>>>>>> >> I'm not sure about this.  Whatever we decide on, it should
>>>>>> >> hopefully
>>>>>> >> be
>>>>>> >> compact.
>>>>>> >>
>>>>>> >>>
>>>>>> >>>
>>>>>> >>> There is also a PR awaiting approval by John Regan that deals with
>>>>>> >>> this
>>>>>> >>> behavior which will allow users to easily specify whatever label
>>>>>> >>> they want
>>>>>> >>> for the colorbar, but I thought having a sensible default was
>>>>>> >>> appropriate as
>>>>>> >>> well.
>>>>>> >>>
>>>>>> >>> Anyway, what do people think about these potential changes?
>>>>>> >>>
>>>>>> >>> Cameron
>>>>>> >>>
>>>>>> >>>
>>>>>> >>> --
>>>>>> >>> Cameron Hummels
>>>>>> >>> Postdoctoral Researcher
>>>>>> >>> Steward Observatory
>>>>>> >>> University of Arizona
>>>>>> >>> 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
>>>>>> >>
>>>>>> >
>>>>>> >
>>>>>> >
>>>>>> > --
>>>>>> > Cameron Hummels
>>>>>> > Postdoctoral Researcher
>>>>>> > Steward Observatory
>>>>>> > University of Arizona
>>>>>> > 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
>>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> Cameron Hummels
>>>> Postdoctoral Researcher
>>>> Steward Observatory
>>>> University of Arizona
>>>> 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
>>>>
>>>
>>>
>>>
>>> --
>>> Cameron Hummels
>>> Postdoctoral Researcher
>>> Steward Observatory
>>> University of Arizona
>>> 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
>
> _______________________________________________
> 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