fix: depend on vector v1.3.1 + trim down coffea vectors #1061
Conversation
|
|
||
| PtEtaPhiMLorentzVectorArray.ProjectionClass2D = TwoVectorArray # noqa: F821 | ||
| PtEtaPhiMLorentzVectorArray.ProjectionClass3D = ThreeVectorArray # noqa: F821 | ||
| PtEtaPhiMLorentzVectorArray.ProjectionClass4D = LorentzVectorArray # noqa: F821 | ||
| PtEtaPhiMLorentzVectorArray.MomentumClass = LorentzVectorArray # noqa: F821 |
There was a problem hiding this comment.
MomentumClass is required for depending on vector v1.3.1 - momentum-ness is now infectious (as discussed in the last awkward-dask meeting.
| assert_record_arrays_equal(a.like(b) + b, b + a.like(b), check_type=True) | ||
| assert_record_arrays_equal(a.like(c) + c, c + a.like(c), check_type=True) | ||
| assert_record_arrays_equal(b.like(c) + c, c + b.like(c), check_type=True) |
There was a problem hiding this comment.
We finally see the type errors on adding 2 vectors of different geometric dimensions as now this is handled on the scikit-hep/vector side.
| @awkward.mixin_class_method(numpy.divide, {numbers.Number}) | ||
| def divide(self, other): | ||
| """Divide this vector by a scalar elementwise using its cartesian components | ||
| This is realized by using the multiplication functionality""" | ||
| return self.multiply(1 / other) | ||
|
|
There was a problem hiding this comment.
I've added the divide method in PtEtaPhiMLorentzVector and PtEtaPhiELorentzVector classes to preserve the coordinate names. I have not trimmed down the multiply and negative methods of these classes for the exact same reason.
Vector automatically canonicalizes coordinate names from momentum synonyms to generic ones, and I think we would not want that here.
There was a problem hiding this comment.
By canonicalize do you mean that it rewrites the underlying awkward RecordArray field names?
There was a problem hiding this comment.
Yes. For vector, it is always canonicalized -
In [2]: v = vector.zip({"phi": [1, 2], "pt": [2, 3], "eta": [3, 4], "m": [4, 5]})
In [3]: v.fields
Out[3]: ['rho', 'phi', 'eta', 'tau']but, for coffee vectors, the fields are rewritten (if scikit-hep/vector methods are used) -
In [3]: a = ak.zip(
...: {
...: "pt": [[1, 2], [], [3], [4]],
...: "eta": [[1.2, 1.4], [], [1.6], [3.4]],
...: "phi": [[0.3, 0.4], [], [0.5], [0.6]],
...: "energy": [[50, 51], [], [52], [60]],
...: },
...: with_name="PtEtaPhiELorentzVector",
...: behavior=vector.behavior,
...: )
In [4]: a.fields
Out[4]: ['pt', 'eta', 'phi', 'energy']
In [5]: (a * 2).fields
Out[5]: ['rho', 'phi', 'eta', 't']
Saransh-cpp
changed the title
Hi @lgray @nsmith-
I am not sure if this is the best way to make changes to the
use_scikithep_vectorbranch. I cannot push the commit directly to the coffee branch as I'm not a collaborator.This PR removes redundant methods in coffee vector classes and makes coffee vectors use the scikit-hep/vector methods.