smash-transport / sparkx

SPARKX - Software Package for Analyzing Relativistic Kinematics in Collision eXperiments
https://smash-transport.github.io/sparkx/
GNU General Public License v3.0
5 stars 0 forks source link

Sass/rename particle functions #263

Closed nilssass closed 1 month ago

nilssass commented 1 month ago

In this PR I do two things:

1. Renaming Member Functions

In Particle we have chosen function names that are suboptimal and not intuitive to find or they contained parts in their name that do not contain any useful information. I have renamed them to make them more precise and easier to find. They are:

Old Function Name New Function Name
momentum_rapidity_Y() rapidity()
spatial_rapidity() spacetime_rapidity()
spatial_rapidity_cut() spacetime_rapidity_cut()
pt_abs() pT_abs()
pt_cut() pT_cut()
compute_mass_from_energy_momentum() mass_from_energy_momentum()
compute_charge_from_pdg() charge_from_pdg()

2. Introducing Helper Functions

In tests/test_JetAnalysis.py we were comparing two nested lists using

# Compare the two contents
assert test_jet_finding_content == generated_jet_finding_content

However, this fails on different machines because of numerical precision when comparing floats. I have introduced a new file tests/HelperFunctions.py that contains a function to compare two lists taking into account the limits of numerical precision. This function is now used

Hendrik1704 commented 1 month ago

We should be careful with changing the name of functions in Particle.

These are just some thoughts coming to my mind after seeing the comment of your PR. Haven't had time to look at the code and the new names yet.

nilssass commented 1 month ago

Absolutely, the Changelog needs of course to be adjusted, this I will add. Besides that I have carefully checked for all occurrences of the functions in the entire code base and made sure that all tests pass. I see the point that renaming function names will potentially break user scripts. That's why I want to clean them up all together once, especially for version 2.0, where a user might expect some bigger changes. With this PR we should be done with name changes and all names are such that they are easily findable if an user works with an IDE.

nilssass commented 1 month ago

@NGoetz this PR is ready for a second review

nilssass commented 1 month ago

@Hendrik1704 @NGoetz Once Niklas reviewed the PR, please, do not yet merge it. I am just modifying the open PR #264 as I realized how many code duplications we have in Filters.py. I am just cleaning it up and I think it will be more efficient to first merge #264 once it is ready