Naming conventions

[nsmith-]

Hi, not sure where to put this but I wanted to check that the work I did for coffea vectors will transfer seamlessly to this when it is ready. I have the following classes (implementation here) and was wondering if you could review the naming convention and see if it matches the plans for this library, at least for the member attributes. I see that the ROOT TVectors prefer to avoid using r and rho for ambiguity's sake since depending where you look you find r for the 2D polar radius and rho for the 3D distance and vice-versa (e.g. TVector vs. https://en.wikipedia.org/wiki/Spherical_coordinate_system)
In addition, I wonder if the inheritance approach will be taken for this library or not (I'm not sure myself if it makes sense) and if there are also plans for Point and "Point with origin" classes?

[nsmith-]

P.S. I just pushed a much more thoroughly documented module to help with this.

P. P. S. I would happily make a PR with the contents of that module so non-coffea users can have something to get started with on awkward1. I feel like awkward without vector mixins is like cake without ice cream

[donalhill]

Hi all - I just wanted to chip in here to say that some FCC studies that I'm starting here with uproot and awkward will benefit a lot from vector. I anticipate a good amount of uptake from people just starting FCC studies, so it will be great to have vector as part of the setup.

On this note - it may be that Coffea can help us too, rather than me reinventing the wheel! I'm coming to this work from a flavour physics perspective, so building lots of exclusive decay modes and studying the momentum, impact parameter, lifetime resolutions e.t.c.

[henryiii]

I finally released Hist 2.0, so this will be my next (large) priority. I have a talk to write for Wednesday, so likely after that? Help and example appreciated!

[eduardo-rodrigues]

For reference - when discussing naming conventions, hence the API, we should not forget all that we're done in the past and is still available in the scikit-hep metapackage, see https://github.com/scikit-hep/scikit-hep/tree/master/skhep/math . I will be involved in the discussions for sure :-).

[henryiii]

Let's start by discussing .dot. Should we:

1. Provide only one form. v1.dot(v2).
2. Provide an alternate spelling, v1 @ v2.
3. Provide an alternate spelling, v1 * v2.

For now, I've implemented 3, but I might rather 1 or 2. Thoughts? Please reply inline, new discussion comments should be about different aspects of the API.

[jpivarski]

At first, I would say, "Yes!" In analysis code, I'd usually opt for the binary operator, but in formal programming, I do prefer words for clarity and uniformity. For instance, I definitely replace & and | with numpy.bitwise_and and numpy.bitwise_or (or the logical_and and logical_or, since they are different and sometimes the difference is important). I did exactly this on a pull request I received a few months ago. As single-character operators, the operation was too easy to miss seeing.

Having add and multiply (which are the spellings of the NumPy ufuncs) as vector methods would make a lot of sense, allowing an expression in formal code to uniformly consist of method applications. The direction of operator precedence would then be strictly left-to-right, which also helps. There wouldn't be a distinct add for the scalar and vector cases: it is conventional to broadcast. (Dot product, by contrast, isn't broadcasting.)

However, NumPy doesn't do that. NumPy has a dot method that is equivalent to @, and of course there are numpy.add and numpy.multiply ufuncs, but not methods. That breaks pattern with a lot of other methods that are also free-standing functions, but the NumPy project is trying to cut down on that duplication anyway. So maybe there's enough of a precedent for addition and scalar multiplication to be done with binary operators that add and multiply methods don't need to exist.

I'd say that's your call: consistently adding an English method name for every operation or following NumPy in doing dot but not add and multiply.

[henryiii]

I'd like to go with avoiding duplication. I don't see anything wrong with calling v1.__add__(v2) if someone wants to be explicit and/or avoid precedence issues. It's a little long with the dunder's, but it's clear and fully mandated by Python rather than having to look up whether a method is called mul or multiply or multiply_scalar. dot is special because you are actually doing a dot product, not a matrix multiplication or element-wise multiplication, so only providing something like __mat__ is unintuitive.

This might be partially why NumPy does not do this - why provide .add when .__add__ is identical and the idiomatic way to spell it in Python?

[jpivarski]

Even though the special methods (__add__ and such) have a meaning that is guaranteed by Python, I think they fall under the "not public API; do not use" rule and therefore I don't use them in formal code.

Anyway, I don't have strong feelings on adding the add and multiply methods, particularly because NumPy doesn't, and that's an easy convention-setting rule to apply.

The dot is special for an additional reason: it should have symmetry with cross and there's no Python operator for a vector cross-product.

[nsmith-]

I'm against the proposition that dunder methods (__add__) fill the role of an instance method here, as it appears private. For what its worth, I did include v.add and v.multiply, keeping the naming suggested by the equivalent numpy ufunc that the binary method would call (e.g. https://github.com/CoffeaTeam/coffea/blob/master/coffea/nanoevents/methods/vector.py#L112 ) but personally, I wouldn't be against having a vector namespace with free functions that apply pairwise.

[henryiii]

Dunder methods are definitely not private - you can't access the __name__ of a class any other way (AFAIK). And I always use f"{self.__class__.__name__}()" instead of f"{type(self).__name__}()" - I prefer the fewer parenthesis, and "{self.__class__.__name__}()".format(self=self) is supported, while you can't do that inline with type. There's also super().__init__(), etc. Python literally treats these different from double-underscore but not ending in double underscore names, which are private (name mangled outside the class, for inheritance purposes mostly). Double underscore just means they are reserved for a specific meaning - add in this case. Which is the correct meaning.

My point was that if you do need to call a method, it's quite safe to call __add__, and adding a .add is entirely optional - and most languages support overloading + anyway, which is a more natural way to write it. I really want to keep duplication down in the core library, since we already have a ton of duplication anyway, for the initial five backends.

@mayou36 might have a strong opinion, though - the TensorFlow method of writing things may tend to look like LorentzZYXT.__add__(v1, v2) / LorentzZYXT.add(v1, v2) - I'm mildly okay with the former, and I don't like having to learn something library specific (add) vs. something standard (__add__), but it does look odd if that's the main way to use it.

naming suggested by the equivalent numpy ufunc that the binary method would call

In NumPy, this is called np.add or array.__add__, there is no array.add.

vector namespace with free functions that apply pairwise.

In Python 3, you can use the class methods as free functions, so that's how these will be used if you want free functions. There will be no duplicated namespace like NumPy has. We have far too much duplication as it is. :)

[henryiii]

For these discussions, the current Protocol in src/vector/protocols/lorentz.py, and you can also look at the parametrized tests in tests/test_simple_param.py (static link, will likely rename soon).