[yt-dev] Field naming proposal
Ben Thompson
bthompson2090 at gmail.com
Mon Dec 29 13:52:53 PST 2014
Hey all.
Nathan, thank you for setting this up, Cameron thank you for clearly
outlining the naming conventions, and sorry to you all if I have been a bit
quiet recently.
I am for option #1 which to me feels a bit more natural to go particle ->
vector field -> coordinate system -> coordinate
As Britton has said, it would be best to finalise a decision in the new
year (maybe at the end of the first full week? say the 8th?).
This PR in question also corrects numerical computation in the
particle_spherical co-ordinate system too as well as updatin the field
naming YTEP, so is also important to get out in it's own right.
Ben
On Wed, Dec 24, 2014 at 5:14 AM, Sam Skillman <samskillman at gmail.com> wrote:
> Hi all, I agree with Britton here that it would be good to table this
> until folks have time to read through this carefully. Thanks, Sam
>
>
> On Tue Dec 23 2014 at 2:40:41 PM Britton Smith <brittonsmith at gmail.com>
> wrote:
>
>> I choose option #1.
>>
>> Also, let's not be too quick to make big decisions here. Many people are
>> on break right now and so are unavailable, or are wanting to be.
>>
>> On Tue, Dec 23, 2014 at 3:42 PM, Nathan Goldbaum <nathan12343 at gmail.com>
>> wrote:
>>
>>>
>>>
>>> On Tue, Dec 23, 2014 at 1:33 PM, Matthew Turk <matthewturk at gmail.com>
>>> wrote:
>>>
>>>> Hi Cameron,
>>>>
>>>> On Tue, Dec 23, 2014 at 11:56 AM, Cameron Hummels <chummels at gmail.com>
>>>> wrote:
>>>>
>>>>> Hi Nathan,
>>>>>
>>>>> Thanks for your hard work on this PR (along with Ben Thompson). The
>>>>> naming convention that I suggested in the issue a few weeks back (
>>>>> https://bitbucket.org/yt_analysis/yt/issue/947/consistent-field-naming-for-spherical-and)
>>>>> and in the discussion on your PR also matches with past convention. It is
>>>>> slightly different than what you propose, but seems (to me at least) to be
>>>>> more easy to read because the adjective comes before the noun (e.g.
>>>>> spherical position) instead of the reverse (e.g. position spherical).
>>>>>
>>>>>
>>>> I'm neutral to both of these, in that I am broadly neutral about the
>>>> increasingly nested set of modifiers. If forced, I think I'd go with your
>>>> proposed convention.
>>>>
>>>
>>> That's two votes against my convention (Cameron and Matt). If no one
>>> else pipes up in favor of my convention in the next day or so, I'll go
>>> ahead and create a YTEP PR and update my PR to match. This means we need
>>> to deprecate fewer fields, so it's probably simpler in the end...
>>>
>>>
>>>>
>>>> What would be a lot nicer, in my opinion, would be if we had a way to
>>>> do this more generically. Like,
>>>>
>>>> with data_object.rotate( ... ):
>>>> prof1d = create_profile(data_object, "particle_position_x",
>>>> "particle_mass")
>>>>
>>>
>>> This is a much nicer syntax. We should consider this for a future
>>> release. If someone puts together a prototype for using data objects with
>>> context managers like this, I think we can have a big usability win for a
>>> lot of use cases.
>>>
>>> Unfortunately we would probably still need to accept the names with
>>> modifiers for backward compatibility.
>>>
>>>
>>>>
>>>> and then just get rid of all the nested modified field names. But I
>>>> don't really think this is feasible.
>>>>
>>>> -Matt
>>>>
>>>>
>>>>> Where proposed naming convention #1 is:
>>>>>
>>>>> (field_type, "<particle?>_<position/velocity>_<coordinate_system>_<coordinate>")
>>>>> e.g. ('all', 'particle_position_spherical_phi')
>>>>>
>>>>> Proposed naming convention #2 is:
>>>>>
>>>>> (field_type, "<particle?>_<coordinate_system
>>>>> >_<position/velocity>_<coordinate>") e.g. ('all',
>>>>> 'particle_spherical_position_phi')
>>>>>
>>>>> Here are all of the relevant gas and particle fields each convention:
>>>>>
>>>>> *Cartesian* (convention #1 & #2 are the same)
>>>>>
>>>>> ('index', 'x')
>>>>> ('index', 'y')
>>>>> ('index', 'z')
>>>>> ('gas', 'velocity_x')
>>>>> ('gas', 'velocity_y')
>>>>> ('gas', 'velocity_z')
>>>>>
>>>>> ('all', 'particle_position_x')
>>>>> ('all', 'particle_position_y')
>>>>> ('all', 'particle_position_z')
>>>>> ('all', 'particle_velocity_x')
>>>>> ('all', 'particle_velocity_y')
>>>>> ('all', 'particle_velocity_z')
>>>>>
>>>>> Convention #1 & #2 differ for the fields of cartesian position
>>>>> relative to the 'center' and 'normal' field parameters for the origin and
>>>>> z-vector:
>>>>>
>>>>> #1 vs #2
>>>>> ('all', 'particle_position_relative_x') vs. ('all',
>>>>> 'particle_relative_position_x')
>>>>> ('all', 'particle_position_relative_y') vs. ('all',
>>>>> 'particle_relative_position_y')
>>>>> ('all', 'particle_position_relative_z') vs. ('all',
>>>>> 'particle_relative_position_z')
>>>>> ('all', 'particle_velocity_relative_x') vs. ('all',
>>>>> 'particle_velocity_position_x')
>>>>> ('all', 'particle_velocity_relative_y') vs. ('all',
>>>>> 'particle_velocity_position_y')
>>>>> ('all', 'particle_velocity_relative_z') vs. ('all',
>>>>> 'particle_velocity_position_z')
>>>>>
>>>>> *Spherical*:
>>>>> #1 vs #2
>>>>>
>>>>> ('index', 'spherical_phi')
>>>>> ('index', 'spherical_radius')
>>>>> ('index', 'spherical_theta')
>>>>> ('gas', 'velocity_spherical_phi') vs ('gas',
>>>>> 'spherical_velocity_phi')
>>>>> ('gas', 'velocity_spherical_theta') vs ('gas',
>>>>> 'spherical_velocity_theta')
>>>>> ('gas', 'velocity_spherical_radius') vs ('gas',
>>>>> 'spherical_velocity_radius')
>>>>>
>>>>> ('all', 'particle_position_spherical_phi') vs ('all',
>>>>> 'particle_spherical_position_phi')
>>>>> ('all', 'particle_position_spherical_theta') vs ('all',
>>>>> 'particle_spherical_position_theta')
>>>>> ('all', 'particle_position_spherical_radius') vs ('all',
>>>>> 'particle_spherical_position_radius')
>>>>> ('all', 'particle_velocity_spherical_phi') vs ('all',
>>>>> 'particle_spherical_velocity_phi')
>>>>> ('all', 'particle_velocity_spherical_theta') vs ('all',
>>>>> 'particle_spherical_velocity_theta')
>>>>> ('all', 'particle_velocity_spherical_radius') vs ('all',
>>>>> 'particle_spherical_velocity_radius')
>>>>>
>>>>> *Cylindrical*:
>>>>> #1 vs #2
>>>>>
>>>>> ('index', 'cylindrical_phi')
>>>>> ('index', 'cylindrical_radius')
>>>>> ('index', 'cylindrical_theta')
>>>>> ('gas', 'velocity_cylindrical_phi') vs ('gas',
>>>>> 'cylindrical_velocity_phi')
>>>>> ('gas', 'velocity_cylindrical_theta') vs ('gas',
>>>>> 'cylindrical_velocity_theta')
>>>>> ('gas', 'velocity_cylindrical_radius') vs ('gas',
>>>>> 'cylindrical_velocity_radius')
>>>>>
>>>>> ('all', 'particle_position_cylindrical_phi') vs ('all',
>>>>> 'particle_cylindrical_position_phi')
>>>>> ('all', 'particle_position_cylindrical_theta') vs ('all',
>>>>> 'particle_cylindrical_position_theta')
>>>>> ('all', 'particle_position_cylindrical_radius') vs ('all',
>>>>> 'particle_cylindrical_position_radius')
>>>>> ('all', 'particle_velocity_cylindrical_phi') vs ('all',
>>>>> 'particle_cylindrical_velocity_phi')
>>>>> ('all', 'particle_velocity_cylindrical_theta') vs ('all',
>>>>> 'particle_cylindrical_velocity_theta')
>>>>> ('all', 'particle_velocity_cylindrical_radius') vs ('all',
>>>>> 'particle_cylindrical_velocity_radius')
>>>>>
>>>>>
>>>>> So what does the community think would be the best system here? #1 or
>>>>> #2? Either way it goes, I think this is a big improvement over the
>>>>> previous naming convention that had general inconsistencies.
>>>>>
>>>>> Cameron
>>>>>
>>>>>
>>>>> On Tue, Dec 23, 2014 at 11:01 AM, Nathan Goldbaum <
>>>>> nathan12343 at gmail.com> wrote:
>>>>>
>>>>>> Hi all,
>>>>>>
>>>>>> I've just issued a pull request that bears some developer discussion.
>>>>>>
>>>>>> Right now yt is a bit of a wild west in terms of the field naming
>>>>>> convention for fields that reference a coordinate system. See for example,
>>>>>> see issue 947:
>>>>>>
>>>>>>
>>>>>> https://bitbucket.org/yt_analysis/yt/issue/947/consistent-field-naming-for-spherical-and
>>>>>>
>>>>>> I'd like to propose a naming convention for fields that reference a
>>>>>> coordinate system. Gas and particle fields should be of the form:
>>>>>>
>>>>>> (field_type, "<particle?>_<vector_field_name>_<coordinate>")
>>>>>>
>>>>>> while index fields for coordinates should be of the form:
>>>>>>
>>>>>> ("index", "<coordinate>")
>>>>>>
>>>>>> This fits within our existing field naming convention for cartesian
>>>>>> coordinates, e.g.:
>>>>>>
>>>>>> ("gas", "velocity_x")
>>>>>> (ptype, "particle_velocity_y")
>>>>>>
>>>>>> as well as our convention for index coordinate fields, e.g.:
>>>>>>
>>>>>> ("index", "x")
>>>>>> ("index", "spherical_theta")
>>>>>>
>>>>>> This means that index fields do not need to explicitly reference
>>>>>> themselves as positions. So we *won't* have field names like:
>>>>>>
>>>>>> ("index", "position_x")
>>>>>>
>>>>>> I don't like the above construction because it's a bit redundant
>>>>>> ("index" implies that we are talking about a position or something similar).
>>>>>>
>>>>>> Some existing field names will need to be changed to fit this. In
>>>>>> particular, some of the index fields will need to be renamed to be more
>>>>>> verbose ("index", "spherical_r") becomes ("index", "spherical_radius") and
>>>>>> (ptype, "particle_spherical_position_radius") becomes (ptype,
>>>>>> "particle_position_spherical_radius").
>>>>>>
>>>>>> Wherever an existing field name needs to change, I propose we mark
>>>>>> the existing field name for deprecation, stub it out, and make it an alias
>>>>>> for the field with the new field name. In a future release, we can then
>>>>>> remove the deprecated fields.
>>>>>>
>>>>>> I've implemented this for the particle fields (for the most part) in
>>>>>> PR 1378:
>>>>>>
>>>>>> https://bitbucket.org/yt_analysis/yt/pull-request/1378
>>>>>>
>>>>>> I'm happy to update the field naming YTEP if this proposed field
>>>>>> naming scheme gets approval in this thread.
>>>>>>
>>>>>> What do you all think? Question, concerns?
>>>>>>
>>>>>> -Nathan
>>>>>>
>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> 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
>>
>
> _______________________________________________
> 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/20141229/5b44d571/attachment.html>
-------------- next part --------------
_______________________________________________
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