IAP GITLAB

Skip to content
Snippets Groups Projects

Improved command line parsing for main corsika binary

Merged Remy Prechelt requested to merge rprechelt/command-line-parsing into master

This MR implements a completely new command line parser for the corsika example to make it significantly more user friendly for running showers. The parsing is implemented with the CLI11 command line library (provided by Conan); this is hands-down the best CLI library for C++ and is provided as a single header-file (seriously, it's awesome).

This new example exposes more options to the command line as well as performs validation during parsing of the options to make it more resilient.

If you run corsika -h, you will get the following help message:

Simulate standard (downgoing) showers with CORSIKA 8.
Usage: ./bin/corsika [OPTIONS]

Options:
  -h,--help                   Print this help message and exit


Primary:
  -Z INT:INT in [0 - 26] Excludes: --pdg
                              Atomic number for primary
  -A INT:INT in [1 - 58] Needs: -Z Excludes: --pdg
                              Atomic mass number for primary
  -p,--pdg Excludes: -Z -A    PDG code for primary.
  -E,--energy :POSITIVE REQUIRED
                              Primary energy in GeV
  -z,--zenith :INT in [0 - 90]=0 REQUIRED
                              Primary zenith angle (deg)
  -a,--azimuth :INT in [0 - 360]=0
                              Primary azimuth angle (deg)


Library/Output:
  -N,--nevent INT:POSITIVE REQUIRED
                              The number of events/showers to run.
  -f,--filename :PATH(non-existing)=corsika_library REQUIRED
                              Filename for output library.


Misc.:
  -s,--seed :NONNEGATIVE=12351739
                              The random number seed.
  --force-interaction         Force the location of the first interaction.

CLI11 makes this super reliable - arguments can be given in any order with any combination of short and long form arguments.

The following validations are performed by CLI11 during parsing,

  1. The user can provide --pdg for an explicit PDG code, or can provide -A and -Z but not both.
  2. A/Z is currently limited to iron (I'm happy to change this but it seems like a good starting place until we are more confident about C8?) and are checked to be greater than 1 and 0 respectively.
  3. Zenith angle is restricted to between 0 and 90 degrees.
  4. Azimuth angle is restricted to between 0 and 360 degrees.
  5. The number of events is checked to be a positive integer.
  6. CLI11 also checks that the path given for the output doesn't already exist.

This is still a WIP and anyone is welcome to contribute to this MR to add more options. I am planning to add (latitude, longitude, altitude) options once the open IGRF MR is approved as well as some minor cleanup.

If people you have suggestions for options to add, please leave a comment!

Edited by Ralf Ulrich

Merge request reports

Merge request pipeline #4543 passed

Merge request pipeline passed for 09515d45

Test coverage 95.80% (0.00%) from 1 job
Approved by

Merged by Ralf UlrichRalf Ulrich 3 years ago (May 25, 2021 6:57pm UTC)

Merge details

  • Changes merged into master with 4497a2d0.
  • Deleted the source branch.
  • Auto-merge enabled

Pipeline #4551 passed with warnings

Pipeline passed with warnings for 4497a2d0 on master

Test coverage 95.80% (0.00%) from 1 job

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Remy Prechelt changed milestone to %ICRC-2021

    changed milestone to %ICRC-2021

  • Remy Prechelt added 1 commit

    added 1 commit

    • ba698cf4 - Disable explicit test arguments for corsika.

    Compare with previous version

  • Very nice. This is clearly the next step to improve the examples. I am very happy with the proposed changes and have no objections to move in this direction. cli11 is header-only, right?

  • Author Maintainer

    Yes; CLI-11 is header-only. It's exceptionally well-written and compiles extremely quickly (i.e it doesn't add to our measurable compile time as best as I could measure over 50 trials).

    It's typically just a single header-file but it's split into 3 parts for Conan just in case you only want to use different components independently.

    It also has the ability to load configuration files into the options instead of command line flags - this would make implementing my goal of taking a config.yaml from a previous library and setting up CORSIKA in exactly the same state much easier.

  • Indeed. But why does the CI fail right now? Not clear to me.

  • Author Maintainer

    I believe it's the same thing as before when we had to update conan - changes to conanfile.txt (i.e. adding cli11) don't seem to be used in the container. I think this is due to the pre-built packages in the container?

  • I'll check, but in principle, conan will install during cmake any package that was found missing. But maybe this is not transported from one CI job to another... I am not sure.

  • I added cli11 to the containers, let's wait.

  • The code approval procedure is described in the wiki: Code approval procedure wiki

    • The MR is without WIP/Draft status
    • Make sure the most recent CI jobs (config, quality, build_test_example) all run fine with no failures
      • if "check clang-format" failed: the code contributor has to run ./do-clang-format.py --apply eventually with the --all option
      • if "check copyright" failed the code contributor has to run./do-copyright.py --add=20xy
    • Make sure also the jobs with MR-label ready for code review succeed. This includes the optional jobs, in particular 'coverage', 'release-clang-8", "release-u-18.04" and make sure no problems occur. You may have to trigger a pipeline manually to check this.
    • Check in the "coverage" job output that the coverage did not decrease. It should always stay, or increase. If it decreased --> ask contributor to add further needed unit tests, and check coverage report.
    • On the MR page, open the "Open in Web IDE" tool
      • Check if the provided solution solves the Issue, discuss on gitlab
      • Check that all changes are actually related to the issue
      • There are no debug statements left, not even commented out
      • Check all changes for coding rules and guidelines
    • When all above is done:
      • Add MR label "Code Review Finished"
      • Approve (click button)
      • Rebase, if needed. If unclear: discuss!
        • git rebase master (rebase on top of master)
        • git merge master (merge back changes from master)
    Edited by Ralf Ulrich
  • Ralf Ulrich unmarked as a Work In Progress

    unmarked as a Work In Progress

  • Ralf Ulrich approved this merge request

    approved this merge request

  • Ralf Ulrich added 8 commits

    added 8 commits

    Compare with previous version

  • Ralf Ulrich enabled an automatic merge when the pipeline for 09515d45 succeeds

    enabled an automatic merge when the pipeline for 09515d45 succeeds

  • merged

  • Ralf Ulrich mentioned in commit 4497a2d0

    mentioned in commit 4497a2d0

Please register or sign in to reply
Loading