Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add rapidity methods #9

Merged
merged 4 commits into from
Oct 16, 2023
Merged

Add rapidity methods #9

merged 4 commits into from
Oct 16, 2023

Conversation

graeme-a-stewart
Copy link
Member

@graeme-a-stewart graeme-a-stewart commented Oct 13, 2023

Add methods that return the true rapidity of LorentzVectors (E, p) these are defined for both cartesian and cylindrical cases as rap() rapidity().

Add methods for the square of mass and the square of momentum (mass2, pt2) as these can be useful for more specialist calculations (but not exported).

Add methods that return the true rapidity of LorentzVectors (E, p)
these are defined for both cartesian and cylindrical cases as rap()

Add methods for the square of mass and the square of momentum
as these can be useful for more specialist calculations (but not
exported)
@codecov
Copy link

codecov bot commented Oct 13, 2023

Codecov Report

All modified lines are covered by tests ✅

Files Coverage Δ
src/LorentzVectorHEP.jl 100.00% <ø> (ø)
src/cartesian.jl 66.03% <100.00%> (+27.01%) ⬆️
src/cylindrical.jl 95.23% <100.00%> (+0.50%) ⬆️

📢 Thoughts on this report? Let us know!.

README.md Outdated Show resolved Hide resolved
src/cartesian.jl Outdated
mass(lv::LorentzVector) = sqrt(dot(lv, lv))
pt(lv::LorentzVector) = sqrt(muladd(lv.x, lv.x, lv.y^2))
mass2(lv::LorentzVector) = dot(lv, lv)
mass(lv::LorentzVector) = mass2(lv) < 0.0 ? sqrt(-mass2(lv)) : sqrt(mass2(lv))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this negative mass trick universal? what's ROOT's Math::LorentzVector behavior on this

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ROOT adopts this convention for space-like 4-vectors, so that's some precedent

Copy link
Member

@Moelf Moelf Oct 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm trying to think if we should err on the side of not silently produce wrong results

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Depends what one thinks of as being wrong. Negative mass is not ambiguous - but it may be surprising. I'm ok with rolling this back.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I guess I mean if we error, it's trivial for user to do what they want. But not the other way, they may silently get wrong results

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked in the Python Vector package and it also conforms to this behaviour:

In [1]: import vector

In [2]: ep_vec = vector.obj(x=8.1, y=2.2, z=3.3, E=4.4)

In [3]: ep_vec.mass
Out[3]: -7.87273777030583

And I found I missed a - sign on the mass method, oops! Fixed in incoming commit.

This is the same method name as ScikitHEP's Vector package
Add a note that we adopt the ROOT convention of returning a negative
mass for space-like 4-vectors
@Moelf
Copy link
Member

Moelf commented Oct 13, 2023

Go ahead and merge this when you're ready!

@graeme-a-stewart graeme-a-stewart merged commit 6f0e9e6 into main Oct 16, 2023
7 checks passed
@graeme-a-stewart
Copy link
Member Author

Thanks @Moelf - merge early, merge often!

Can we foresee a new release that can be a proper dependency for JetReconstruction.jl?

@Moelf Moelf deleted the add-rapidity branch October 16, 2023 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants