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 periodic masking option to Aperture. #739

Merged
merged 14 commits into from
Oct 17, 2024

Conversation

cemitch99
Copy link
Member

@cemitch99 cemitch99 commented Oct 11, 2024

This PR adds the capability to allow periodic transverse masking of beam particles (as relevant for modeling, e.g. pepperpot diagnostics). This is implemented by adding two optional parameters to the existing Aperture element, representing a horizontal and vertical period with which the aperture is repeated. This addresses Issue #694.

  • add repeat_x and repeat_y aperture inputs
  • update Python bindings
  • update documentation
  • add example

@ax3l ax3l added component: elements Elements/external fields enhancement New feature or request labels Oct 11, 2024
@cemitch99 cemitch99 requested review from n01r and EZoni and removed request for ax3l October 12, 2024 18:31
examples/aperture/README.rst Outdated Show resolved Hide resolved
Copy link
Member

@EZoni EZoni left a comment

Choose a reason for hiding this comment

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

Thanks, looks good to me. I'd let @SchroederSa and @n01r comment on the physics aspects of the PR.

Correct type in example README.
@cemitch99
Copy link
Member Author

Illustration.
Uploading ImpactX_Example_Pepperpot.png…

Copy link
Contributor

@SchroederSa SchroederSa left a comment

Choose a reason for hiding this comment

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

I have little to no understanding of the underlaying code, so I added any thoughts and questions that I had when going through it line by line - I coudn't even find a single typo in the docs :).
Thanks again!
Sarah

particles_lost = series_lost.iterations[0].particles["beam"].to_df()

# compare number of particles
num_particles = 100000
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need that line?
I see, this variable is being used in line 60.
I wonder why this number is hard-coded instead of directly defined as len(initial), as it doesn't matter at which initial number of particles this test is being done.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is only to check that the number of particles generated, and the number of particles remaining after the lattice (including those lost) both coincide with the number specified in the input file. We have this check in all of our CI tests.

Copy link
Contributor

Choose a reason for hiding this comment

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

For the example that users are looking at, it would be nice to add another drift and monitor after the pepperpot to complement the setup of an actual emittance measurement and plot the outcome on the last monitor.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed. For automated testing, the existing test is good because it is simple and checks the functionality implemented. I propose that we keep it as-is for this PR, and we can add a follow-up example that includes a full pepperpot emittance reconstruction. We could rename the existing test aperture_periodic to avoid confusion, and name the follow-up test pepperpot_measurement or something similar.

Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above. Maybe it is worth adding another drift and monitor to represent an actual pepperpot emittance measurement setup.

Copy link
Member Author

Choose a reason for hiding this comment

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

Absolutely! See the above suggestion.

@@ -59,6 +61,8 @@ namespace impactx
Aperture (
amrex::ParticleReal xmax,
amrex::ParticleReal ymax,
amrex::ParticleReal repeat_x,
Copy link
Contributor

Choose a reason for hiding this comment

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

You don't define the default value to be 0. Is that on purpose? I see that this is being done in the .cpp file. I just want to double-check, as you need that definition later in line 116 and there is a default value defined for dx, dy and rotation_degree.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the careful check. The default value of 0 is set on lines 379-380 of InitElement.cpp, which is where we read-in the nominal element parameters and include defaults. Only the error parameters dx, dy, and rotation_degree are treated differently, because they are arguments of the Alignment function.

amrex::ParticleReal dx = m_xmax;
amrex::ParticleReal dy = m_ymax;
// first case: a single aperture
if (m_repeat_x == 0.0 || m_repeat_y == 0.0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

How does a periodic slit then work? Just by making the apertures touch each other in the direction of the slit? As a user, I would probably intuitively define only repeat_x and define an elongated rectangular aperture in y.

So maybe this could be an (m_repeat_x == 0.0 && m_repeat_y == 0.0) ?
And then the slit can be done via an option: (m_repeat_x == 0.0 || m_repeat_y == 0.0)

Copy link
Member Author

@cemitch99 cemitch99 Oct 17, 2024

Choose a reason for hiding this comment

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

Yes, correct. To create an x-periodic array of vertical slits, you would now set repeat_x to the period you want, and set ymax = repeat_y. You make a good suggestion about a way to add more flexibility (for example, a rectangular aperture of finite size that is periodically repeated in only one direction). I just pushed a change to address this.

Generalize periodic aperture to allow periodicity in one direction only.
Rename example:  CMakeLists
Rename example:  README documentation
Copy link
Contributor

@SchroederSa SchroederSa left a comment

Choose a reason for hiding this comment

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

Thank you!
Sarah

@cemitch99 cemitch99 merged commit 7a924cc into ECP-WarpX:development Oct 17, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: elements Elements/external fields enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants