scikit-hep / vector

Vector classes and utilities
https://vector.readthedocs.io
BSD 3-Clause "New" or "Revised" License
79 stars 27 forks source link

Constructing a vector from spherical coordinates #122

Open masonproffitt opened 3 years ago

masonproffitt commented 3 years ago

The README says vectors can be represented in spherical coordinates. It's true that you can get spherical coordinates out, but I don't think this is actually implemented for constructing vectors. That is, you can't pass a three-vector magnitude into a constructor.

AdvaitDhingra commented 3 years ago

So basically a constructor where you provide a r, theta and phi value? @jpivarski

jpivarski commented 3 years ago

The internal representation can't be a 3D radius, phi, and theta, since the coordinates are described as an azimuthal piece, a longitudinal piece, and a temporal piece. The interpretation of each of these coordinates can depend on the previous: the longitudinal can depend on the magnitude of the azimuthal (not z, but theta and eta do) and the temporal can depend on the magnitude of the 3D vector (not t/energy, but tau/mass does). If we used 3D radius in the azimuthal coordinates, the azimuthal calculations would depend on both azimuthal and longitudinal coordinates, which would prevent us from being able to share code across so many coordinate systems.

So, it was a design choice that bought us CodeCogsEqn coordinate systems for the cost of CodeCogsEqn (3) implementations.

I've edited the README—it should never have said that we have "spherical" coordinate systems, given what that's usually taken to mean. (The "r" is not just magnitude in a 2D plane, but magnitude in 3D.) It could be possible to add constructors based on 3D radius, which immediately convert their input to the closest cylindrical coordinate system—i.e. r, phi, whatever → rho, phi, whatever—but that would be a lot of edits to make each of the backends consistent, and this new code would be different in kind from the existing code, as it would perform a transformation on the spot. It's not clear to me how it could be done in the Awkward Array backend, which takes Vector methods as a behavior (no direct constructor). Maybe instead of doing the transformation in the constructor, it's a preprocessing step in all calculations? That, at least, could be done consistently.