Fix kinematics

[HDembinski]

This patch fixes some errors in the formulas of EventKinematics and improves its documentation.

• I replaced most of the explicit formulas with named functions, which improves readability, DRYness, and reduces errors from typos (which were present).
• In the process, I added more tests and realized that our implementation specifies the energy per nucleon when we use a nuclear projectile, not the energy of the nucleus. This is clarified now in the documentation of EventKinematics.
• The mass for a nuclear projectile was wrongly computed, it was the mass of the nucleus instead of the mass of a nucleon. It baffles me that our tests did not catch this somewhere.

I fixed a minor bug in Epos, where the result was always computed in the center-of-mass frame. As a consequence, I needed to replace one of the references.

Other minor changes

• CompositeTarget was moved to impy.util, which is a good place for all of our lowest level utility classes and functions, it is still importable from impy.kinematics, too, so nothing changes from the user perspective
• EventFrame was also moved to impy.util, but is also still importable from kinematics, see previous point
• _normalize_particle is now process_particle in impy.util, since it is used in many places
• Energy units (MeV, TeV, ...) can now be imported also from impy.kinematics in addition to impy.constants, which seems fitting and allows one to save a bit of typing in notebooks
• test_generators.py now only produces images for tests that fail, so you can find them easier. If you want to look at all of them, set DEBUG=1: DEBUG=1 python -m pytest tests/test_generators.py

[afedynitch]

@HDembinski, thanks for doing more nice work. This PR is fairly straight forward. I left a few comments in the files, and below are a few other ones:

• I replaced most of the explicit formulas with named functions, which improves readability, DRYness, and reduces errors from typos (which were present).

• This is indeed good. But I didn’t see any obvious typo, could you point it out? (Because of that’s relevant it might be in the MCEq tables etc.)

• In the process, I added more tests and realized that our implementation specifies the energy per nucleon when we use a nuclear projectile, not the energy of the nucleus. This is clarified now in the documentation of EventKinematics.
  That’s how most of the accelerators specify the energy and it also makes sense, the boost is the same if you use gamma=energy_per_nucleon/m_nucleon or gamma=energy/m_nucleus.
• The mass for a nuclear projectile was wrongly computed, it was the mass of the nucleus instead of the mass of a nucleon. It baffles me that our tests did not catch this somewhere.

• This might not have an effect because the mass is never specified or used if the energy is given in the correct variable, ecm or elab for the corresponding model.

I fixed a minor bug in Epos, where the result was always computed in the center-of-mass frame. As a consequence, I needed to replace one of the references.

• Noticed that. Strange. Is there a test for energy or momentum conservation of the final state particles?

All the other changes are clean improvements. Cheers.

[HDembinski]

This might not have an effect because the mass is never specified or used if the energy is given in the correct variable, ecm or elab for the corresponding model.

Yes, we use the mass to compute momentum from energy, but in probably all tests we always use the energy, and like you said, the models also use energy, except perhaps Dpmjet which wants the full 4-vectors.

[HDembinski]

Noticed that. Strange. Is there a test for energy or momentum conservation of the final state particles?

No, but I know that some models, notably EPOS, violate this to some extend. It is even pointed out in this paper (by other authors) https://inspirehep.net/literature/1859944

I noticed the violation of E/p conservation in EPOS a few years ago when I wrote a reader of CRMC output for the NA61/SHINE simulation software. That reader was not simply copying particles but reproducing the history, which is a bit more complex, so I used E/p conversion of final state particles to test the implementation.

I think a test of E/p conservation is not so necessary for impy, since we are just copying the original MC output, but it cannot hurt to have. It is just does not have high priority on my list.

[HDembinski]

Very strange, I made a small change, and now UrQMD is failing. And I just regenerated the reference.

[HDembinski]

The windows test never finishes either.

[HDembinski]

The tests fail because a small shift in the pseudorapidities moves a delta proton peak by one bin.

[afedynitch]

@HDembinski. The PR would be ready to merge except this UrQMD issue. You mentioned that you regenerated the expectation but the test is still failing. What is your plan on this?

[HDembinski]

@afedynitch I think it is fine to acccept this, since the issue is understood and not of concern. But I wanted to mark the offending test as xfail before merging, to keep the lights green (otherwise it is hard to detect new failures). I did not get around to doing this.

[afedynitch]

@HDembinski we're now getting the XCode 14 bug on the macos-latest builds. It seems that our workers migrated to macos 12 and the XCode14 tool chain that has the weird linker bug we encountered.

The 14.2 version that has the bug fixed will become default on January 16th. How unfortunate is that? :) There is a fix, though

[HDembinski]

Thanks!