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

Add fractional_split method #84

Merged
merged 10 commits into from
Nov 20, 2024
Merged

Add fractional_split method #84

merged 10 commits into from
Nov 20, 2024

Conversation

ChristopherMayes
Copy link
Owner

@ChristopherMayes ChristopherMayes commented Nov 7, 2024

Fractional split will use weights to partition the particles:
image

This is useful for splitting particles into head, core, and tail parts. Here, the 5% of the charge is in the tail, 90% is in the core, and 5 % is in the head:
image

@ChristopherMayes ChristopherMayes marked this pull request as ready for review November 19, 2024 23:08
Copy link
Contributor

@ken-lauer ken-lauer left a comment

Choose a reason for hiding this comment

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

Some adjustments after the removal of int handling and this looks good to me. Could use a unit test though.


Parameters:
-----------
fractions : float, int, or list of float/int
Copy link
Contributor

Choose a reason for hiding this comment

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

If all values have to be between 0 and 1 (exclusive), then fractions can't be int (or list of int)

Copy link
Owner Author

Choose a reason for hiding this comment

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

fixed

pmd_beamphysics/particles.py Outdated Show resolved Hide resolved
pmd_beamphysics/particles.py Outdated Show resolved Hide resolved
pmd_beamphysics/particles.py Outdated Show resolved Hide resolved
pmd_beamphysics/particles.py Outdated Show resolved Hide resolved
Copy link
Contributor

@ken-lauer ken-lauer left a comment

Choose a reason for hiding this comment

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

This looks good, but would you consider adding even a smoke test to the suite (i.e., one that ensures the code itself runs without crashing)?

@ChristopherMayes ChristopherMayes merged commit cbf03e5 into master Nov 20, 2024
14 checks passed
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