impy-project / chromo

Hadronic Interaction Model interface in PYthon
Other
30 stars 7 forks source link

Sibyll* #169

Closed afedynitch closed 1 year ago

afedynitch commented 1 year ago

This PR combines multiple items (sorry for that). The main novelty is SIBYLL* as shown at ICRC by F. Riehn:

  1. Include SIBYLL* (thx @jncots)
  2. When using Ninja for compilation, the FORTRAN files were pre-processed twice, once due to the -cpp flag in CMake, and a second time by Ninja itself that calls the preprocessor by default. Fixing this restores the quick build times on machines with many cores.
  3. The allowed projectiles for all of the models have been reviews and the lists extended where appropriate
  4. Some fixes to UrQMD file list (exclude the includes from f2py)

Opinions desired on:

  1. If the SIBYLL* subclasses should be called using the underscore or camel case for consistency?
  2. If I should put effort in using only one source file for SIBYLL2.3d and SIBYLL. In principle, one can restore original SIbyll2.3d behaviour from the sibyll source file.  
jncots commented 1 year ago

I think that:

HDembinski commented 1 year ago

SIBYLL* is an unfortunate naming choice for us. I agree with @jncots that CapWords style for the new classes is the way to be PEP8 compliant, see https://peps.python.org/pep-0008/#class-names

afedynitch commented 1 year ago

@HDembinski I fixed the issues that you pointed out.

Further, I updated Sibyll* to patch release 2 that came in today with a bugfix. After your comments are resolved, I would like to merge this with to_release branch instead of main to bundle the numerous open PRs.

afedynitch commented 1 year ago

I implemented the comments by @jncots.

Also, I fixed the cross section readout for nuclei. It was necessary to include more categories into CrossSectionData to accommodate the special categories for nuclei, such as "production" and "quasielastic". I updated the tests to accomodate these changes. This will close #130 .