This PR is a replacement for @Benjamin Thompson's PR 1308. I did some big updates based on his work, and the discussion in that PR was starting to get really lengthy, so I've reissued the PR here.
This PR does two main things:
The fields currently in yt with names like particle_spherical_position_radius return incorrect results. See #966. This PR ensures these fields return correct results.
Adds new particle_position_relative and particle_velocity_relative fields. This makes it possible to get particle positions and velocities in a rotated reference frame.
The naming system for particle vector fields has never been codified. In this PR I implement a proposed naming system. Particle vector fields will be of the form:
particle_<vector field name>_coordinate
The main advantage is that this matches our current naming system for cartesian coordinates (e.g. particle_position_x, particle_velocity_y. So the spherical radius position field becomes particle_position_spherical_radius.
Where there are fields that are currently in yt that have names that do not match this naming convention, I've stubbed out their implementation and made them aliases for the fields with names following my proposed naming convention. I've also marked these stub fields as deprecated in their docstrings.
When no field already existed (e.g. the position fields in cylindrical coordinates), I've simply added the new fields following my proposed naming convention without adding fields that would immediately need to be deprecated.
I'm confident that these are all giving correct results. See this notebook:
Note that this notebook uses some work-in-progress machinery from PR 1377.
I'm not sure whether this should go in before 3.1. It's a significant change that includes some new code and fields, but it does correct some big issues with the spherical particle position fields. I would very much appreciate comments on that front from PR reviewers.
Your proposed naming scheme for particle fields has direct impacts for corresponding gas fields.
For example, by choosing to have particle fields as (as you do in this PR):
the equivalent gas field would be
Whereas by choosing the particle fields to be (as was done previously, and suggested in the issue above):
the gas fields would be:
Admittedly, both styles seem to be somewhat awkward constructions, but both gas fields and particle fields seemed to me to be slightly less awkward for the second set. That said, I'm open to changing it, I just wanted everyone to realize the consequences this proposed change would have for other fields.
OK, so we disagree on the convention, which is fine, but I think other people should vote on the new proposed convention. Ultimately, I don't care that much, but I just want consistency between the various parts of the code so we don't end up with inconsistent field naming as we've sometimes had in the past as the code grew organically.
@Nathan Goldbaum I think also cylindrical_r and spherical_r in the gas fields need to be replaced with cylindrical_radius and spherical_radius... But for the most parts, the rest does match this convention that we have.
I just wanted to get feedback from the other devs on which is the best naming convention to use between the two proposed. When there is general consensus, I am happy to approve this PR, since it adds new functionality, removes broken functionality, makes a consistent field naming scheme, and adds documentation.