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: configurable particle gun #1093

Closed

Conversation

asalzburger
Copy link
Contributor

This is my first attempt of updating the python scripts to be more flexible if needed.

Also tagging: @timadye

@asalzburger asalzburger added Improvement Changes to an existing feature Component - Examples Affects the Examples module labels Dec 3, 2021
@asalzburger asalzburger added this to the next milestone Dec 3, 2021
@asalzburger asalzburger self-assigned this Dec 3, 2021
Comment on lines +41 to +42
particleConfig: list
partilce configuration: number of particles, particle type, charge flip
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Argubaly this could be separate arguments, like numParticles, particlePdg and particleCargeFlip or something.

@codecov
Copy link

codecov bot commented Dec 3, 2021

Codecov Report

Merging #1093 (9f5c771) into main (dd36b16) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1093   +/-   ##
=======================================
  Coverage   48.61%   48.61%           
=======================================
  Files         341      341           
  Lines       17499    17499           
  Branches     8261     8261           
=======================================
  Hits         8508     8508           
  Misses       3218     3218           
  Partials     5773     5773           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dd36b16...9f5c771. Read the comment docs.

@timadye
Copy link
Contributor

timadye commented Dec 3, 2021

Thanks, @asalzburger! I'll give it a go.

def runParticleGun(
outputDirCsv = None,
outputDirRoot = None,
pConfig = [1 * u.GeV, 10 * u.GeV, True],
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better named momentumConfig

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewing your own code? 😄

@robertlangenberg
Copy link
Contributor

Changes will be incorporated into other PR

timadye pushed a commit to timadye/acts that referenced this pull request Dec 23, 2021
timadye pushed a commit to timadye/acts that referenced this pull request Dec 23, 2021
@paulgessinger paulgessinger modified the milestones: next, v16.0.0 Jan 13, 2022
timadye pushed a commit to timadye/acts that referenced this pull request Feb 3, 2022
robertlangenberg added a commit that referenced this pull request Feb 8, 2022
* configurable particle gun

* Python particle_gun and fatras examples can now be chained

* `addParticleGun` and `addFatras` adds the required modules to the `Sequence`.
* `runParticleGun` and `runFatras` rewritten to use `add*` to reproduce exactly the previous behaviour.
* `ROOT_HASH_CHECKS=on pytest` run as before without change.
* Optionally `runParticleGun` and `runFatras` can now accept `outputDir` as `pathlib.Path` or `str` (like `add*`)
* Other Python examples not yet changed. Let's see how this one goes down.

* rename `pConfig`->`momentumConfig` as per Andi's comment in #1093

* use Path where possible (#1128 (comment))

* use `namedtuple` to pass particle gun parameters

* Keep ParametricParticleGenerator defaults if not specified by caller. Rely on defaults in examples.

* allow `addParticleGun(MomentumConfig())` etc to be specified without keyword

Co-authored-by: Andreas Salzburger <[email protected]>
Co-authored-by: Tim Adye <[email protected]>
Co-authored-by: robertlangenberg <[email protected]>
kodiakhq bot pushed a commit that referenced this pull request Apr 1, 2022
Update the Python in `Examples/Scripts/Python` to allow configuration of various full-chain examples.

Continuing the work of #1098 in order to complete the proposal originally discussed in #1093.
paulgessinger pushed a commit to paulgessinger/acts that referenced this pull request Apr 11, 2022
Update the Python in `Examples/Scripts/Python` to allow configuration of various full-chain examples.

Continuing the work of acts-project#1098 in order to complete the proposal originally discussed in acts-project#1093.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component - Examples Affects the Examples module Improvement Changes to an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants