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

feat: generate eicrecon commands with a YAML file #65

Merged
merged 4 commits into from
May 18, 2023
Merged

feat: generate eicrecon commands with a YAML file #65

merged 4 commits into from
May 18, 2023

Conversation

c-dilks
Copy link
Member

@c-dilks c-dilks commented May 2, 2023

Briefly, what does this PR introduce?

Use a yaml configuration file to set configuration parameters for EICrecon.

The hard-coded parameters in recon.sh are moved to configuration files in config/. Use the new script recon.rb together with one of these yaml files to run EICrecon. Deprecated Juggler support in recon.sh is moved to a new wrapper script, juggler.sh (recon.sh has been renamed to juggler.sh, and its EICrecon support has been removed).

This allows us to (a) run with parameters on the EICrecon IRT development branch irt-algo or irt-algo-stable, (b) run with parameters for EICrecon main, or (c) run with any other set of custom parameters.

This also serves as a proof of concept for configuration file parsing, where the configuration structure is tree-like:

Example output of recon.rb -c config/recon_sandbox.yaml

EICRECON ARGUMENTS: [
  -PDRICH:DRICHRawHits:seed="5",
  -PDRICH:DRICHRawHits:hitTimeWindow="20.0*ns",
  -PDRICH:DRICHRawHits:pixelSize="3.0*mm",
  -PDRICH:DRICHTracks:Aerogel:numPlanes="5",
  -PDRICH:DRICHTracks:Gas:numPlanes="10",
  -PDRICH:DRICHIrtCherenkovParticleID:numRIndexBins="100",
  -PDRICH:DRICHIrtCherenkovParticleID:Aerogel:zbins="5",
  -PDRICH:DRICHIrtCherenkovParticleID:Aerogel:referenceRIndex="1.019",
  -PDRICH:DRICHIrtCherenkovParticleID:Gas:zbins="10",
  -PDRICH:DRICHIrtCherenkovParticleID:Gas:referenceRIndex="1.00076",
  -PDRICH:ExampleArray="1,2,3,4,5",
  -Pplugins="benchmarks_pid,janadot",
  -Ppodio:output_include_collections="DRICHHits,DRICHRawHits,DRICHRawHitsAssociations,DRICHAerogelTracks,DRICHGasTracks,DRICHAerogelIrtCherenkovPa
rticleID,DRICHGasIrtCherenkovParticleID,DRICHMergedCherenkovParticleID,ReconstructedChargedParticles,ReconstructedChargedParticleAssociations,Reco
nstructedChargedParticlesWithDRICHPID,ReconstructedChargedParticleAssociationsWithDRICHPID",
  -Peicrecon:LogLevel="info",
  -Prichgeo:LogLevel="info",
  -Pbenchmarks_pid:LogLevel="info",
  -PDRICH:DRICHRawHits:LogLevel="info",
  -PDRICH:DRICHTracks:LogLevel="info",
  -PDRICH:DRICHIrtCherenkovParticleID:LogLevel="info",
  -PDRICH:DRICHMergedCherenkovParticleID:LogLevel="info",
  -Pjana:nevents="0",
  -Pjana:debug_plugin_loading="1",
  -Pacts:MaterialMap="calibrations/materials-map.cbor",
  -Ppodio:output_file="out/rec.root",
  -Phistsfile="out/rec.ana.root",
  out/sim.root,
]

What kind of change does this PR introduce?

  • Bug fix (issue #__)
  • New feature (issue #__)
  • Documentation update
  • Other: __

Please check if this PR fulfills the following:

  • Tests for the changes have been added
  • Documentation has been added / updated
  • Changes have been communicated to collaborators

Does this PR introduce breaking changes? What changes might users need to make to their code?

no

Does this PR change default behavior?

yes:
@chchatte92 and @DelloStritto: use recon.rb instead of recon.sh:

recon.rb -h  # print usage guide
recon.rb     # run default, for IRT (EICrecon must be on branch irt-algo or irt-algo-stable
recon.rb -c config/recon_main.yaml  # run EICrecon on any other branch (for Luigi)

@veprbl
Copy link
Member

veprbl commented May 16, 2023

We should really be looking to move to Gaudi-like steering with Python. Allegedly JChain is intended to facilitate that. The script from this PR is just re-implementation of xargs, from what I see at a glance. (was glancing at the PR description)

@c-dilks
Copy link
Member Author

c-dilks commented May 16, 2023

I would really like to avoid hard-coded configuration, in favor of a machine readable and machine editable file such as yaml or toml, and be independent of any programming language. If one wants to write code to generate or access certain parameters, that code can generate such a configuration file.

@veprbl
Copy link
Member

veprbl commented May 16, 2023

Yes, absolutely, I do like the direction you are pursuing. I should have displayed more excitement in my original message.

Base automatically changed from irt-algo to main May 18, 2023 20:29
fix: log_level parsing

refactor: rename `recon.sh` to `juggler.sh` and update `recon.rb`

new file:   config/recon_main.yaml
@c-dilks c-dilks marked this pull request as ready for review May 18, 2023 21:03
@c-dilks c-dilks merged commit 748a2e0 into main May 18, 2023
@c-dilks c-dilks deleted the recon-rb branch May 18, 2023 21:03
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.

2 participants