ChrPlasmaLens element: Support k<0, and fix uninitialized-variable bug in tracking of reference particle #1030

kyrsjo · 2025-07-04T09:03:06Z

This patch adds support for negative strength (defocusing) for ChrPlasmaLens and ConstF elements. It also fixes a bug (use of uninitialized variable) in reference particle tracking for ChrPlasmaLens.

…ChrPlasmaLens

kyrsjo · 2025-07-04T09:09:39Z

Hm, as soon as I made the PR I realized I forgot to fix tabs / spaces. However the bot seems to have messed up the number of spaces needed.

I'll fix (and probably force push to override its broken fixes).

Looks like the CI is running the tests for me, that's nice :)

…ith Quad when k=0

ax3l · 2025-07-07T16:50:17Z

Thank you for your first PR to ImpactX, @kyrsjo!

Those changes look great, let us structure & test them to make them best documented/portable/covered by CI.
Please simply push additional commits as needed to the PR to update it as needed.

I also started CI and sent you an invite to our GitHub org, please accept to auto-run CI even before your first PR got merged on updates.

Please make sure to provide a detailed description on the change and try to do one change at a time. e.g., if you fix a bug do it in one PR, adding a feature in another. Ideally, if it affects multiple elements as in your case, with unrelated bugs, then please make a PR per element (plasma lens, quad and ConstF here could be each a PR.)
This makes it easier for us to document changes for users and we can also "backport" fixes to older releases, when needed. It also enables us to merge things faster, because they are less complex and orthogonal changes can progress in parallel.

Please feel free to add your name to the author list of the respective files you changed.

If a specific bug is fixed (or feature added), then I usually structure my PR (branch) like this:

a commit that adds a test that triggers the exact bug (to cover it) / covers the new feature
another commit that adds the fix / feature
documentation updates, if needed (usually for new features)

Welcome again, looking forward to merging those! Please let us know if you have any questions ✨

src/elements/ChrPlasmaLens.H

src/elements/ConstF.H

src/elements/Quad.H

src/elements/ChrPlasmaLens.H

kyrsjo · 2025-07-08T08:49:14Z

Hi @ax3l , thanks for the invite and for looking over it! Most fixes are easy, will do. For the "use ternary operator" thing, please see my comments.

I guess I should add tests for negative-k cases of these elements also.

Is it OK that we keep the elements in 1 PR? I'll separate out the m_slice_ds uninitialized stuff to a separate PR.

I see that the apochromat_pl and apochromat_pl.py tests are failing, will look into that.

General comment: I appreciate that you are insisting on small and readable PRs! Having been involved in a few codes myself, especially being the former release manager and git-repo-czar of sixtrack, it's a very good thing.

ax3l · 2025-07-09T23:41:14Z

Thanks a lot @kyrsjo !

Is it OK that we keep the elements in 1 PR? I'll separate out the m_slice_ds uninitialized stuff to a separate PR.

I would really prefer to do one PR for each, because that way we can merge the quad right away and then decide for ConstF and plasma lens if and how much testing we add, too. Also makes the release notes that we auto-generate readable.

I already moved the Quad fix into #1038. Please merge in development here and it will vanish from the diff in this PR.

Please do the same for ConstF 🙏

cemitch99

I made some small comments, and will try to help pin down why the CI tests are currently failing.

src/elements/ChrPlasmaLens.H

for more information, see https://pre-commit.ci

kyrsjo · 2025-08-25T20:40:05Z

I think this is ready for review/merging now, @ax3l & @cemitch99 ?

There are two questions I wonder about (datatype and tolerance) + 1 question I've answered (why negative).

Note that I've made the tests so that I can fit the ConstF tests into the same framework :)

cemitch99 · 2025-09-18T20:45:46Z

examples/active_plasma_lens/run_APL.py

+
+    # Forward-propagate that through the focusing/defocusing lens
+    # from the beginning, ignoring energy spread
+    # [Doesn't give same result as ChrPlasmaLens and ChrQuad for some reason, even when sigpt_0 = 0]


Does this comment still apply? If so, this needs to be addressed.

Hi, this was fixed here:

bb670a1#diff-a95909e2ce0d4e807a3fa1dc18e09c9c822ec1dbb1275b60b056c775609e216eL125

and then I started using the analytical estimate here:
7f06ab2

I will cleanup when I do constF, PR #1041 - the tests should share quite a bit of code.

cemitch99 · 2025-09-18T21:11:26Z

examples/active_plasma_lens/analysis_APL_ChrPlasmaLens.py

+    beta_y = sigy2 / emittance_y
+    alpha_y = -epstrms["position_y"]["momentum_y"] / emittance_y
+
+    return (beta_x, beta_y, alpha_x, alpha_y)


Note: This works, but the values defined here are computed internally and stored as reduced_diagnostics. See this example for an alternative approach: https://impactx.readthedocs.io/en/latest/usage/examples/fodo_space_charge/README.html
It's fine to keep this for now.

examples/active_plasma_lens/plot_APL_ChrPlasmaLens_analytical.py

docs/source/usage/examples.rst

ax3l

Looks great, thank you for this! 🚀 ✨

examples/active_plasma_lens/README.rst

kyrsjo · 2025-09-22T13:29:18Z

Thank you @ax3l and @cemitch99 for merging! I will now proceed to ConstF. Also, PhD student @aellings will now be able to simulate diverging APLs in ImpactX :)

Maybe see you at Elba?

ax3l · 2025-09-22T18:01:22Z

@kyrsjo Awesome, thanks for the contribution!!

Yes, I am at EAAC until Thursday noon, let's catch up!

kyrsjo added 6 commits July 3, 2025 11:25

Compiles?

2be89b0

Fixed the math, no we can defocus with k<0!

de1830f

Initial add of defocusing ChrPlasmaLens

1267d4a

Fix use of uninitialized variable for reference particle tracking in …

61bfa90

…ChrPlasmaLens

Remove forgotten debugging comment

d39ef33

Splelling

b7e83f5

Fix tabs/spaces

2ab5502

kyrsjo force-pushed the APL-negK branch from d90a1ff to 2ab5502 Compare July 4, 2025 09:12

Fix uninitialized variable giving bad results for envelope tracking w…

98abdfd

…ith Quad when k=0

kyrsjo changed the title ~~Negative k in APLs~~ Negative k in APLs + avoid using uninitialized variables Jul 4, 2025

Handle m_g=0 correctly in ChrPlasmaLens

71140f9

ax3l requested review from ax3l and cemitch99 July 7, 2025 16:23

ax3l added the component: elements Elements/external fields label Jul 7, 2025

ax3l reviewed Jul 7, 2025

View reviewed changes

src/elements/ChrPlasmaLens.H Show resolved Hide resolved

ax3l added bug Something isn't working bug: affects latest release Bug also exists in latest release version labels Jul 7, 2025

kyrsjo added 3 commits July 8, 2025 11:54

Use right litteral type in comparisons

71a25b3

Fix indentation

160d29f

litteral types for assignments

72426ee

cemitch99 reviewed Jul 9, 2025

View reviewed changes

src/elements/ChrPlasmaLens.H Show resolved Hide resolved

src/elements/ChrPlasmaLens.H Show resolved Hide resolved

ax3l mentioned this pull request Jul 9, 2025

Fix Quad Transport Map w/ Slicing and Zero Fields #1038

Merged

kyrsjo commented Jul 10, 2025

View reviewed changes

src/elements/ChrPlasmaLens.H Show resolved Hide resolved

Merge branch 'development' into APL-negK

7702878

kyrsjo mentioned this pull request Jul 11, 2025

[WIP] ConstF: Allow kx<0 and ky<0 #1041

Open

1 task

kyrsjo and others added 7 commits August 25, 2025 20:52

Fix error in CMakeLists

d1cff56

Rename run_APL_ChrPlasmaLens.py -> run_APL.py

42cce64

Swap reference values to analytical values

7f06ab2

Catch error

4a70434

Add README.rst for active_plasma_lens_test

52a252f

[pre-commit.ci] auto fixes from pre-commit.com hooks

efca31b

for more information, see https://pre-commit.ci

Hopefully fixing REST syntax

a64508a

kyrsjo and others added 3 commits August 26, 2025 09:16

Merge branch 'development' into APL-negK

199cef0

Merge branch 'development' into APL-negK

fba821b

Merge branch 'development' into APL-negK

4866ef8

cemitch99 reviewed Sep 18, 2025

View reviewed changes

cemitch99 approved these changes Sep 18, 2025

View reviewed changes

cemitch99 reviewed Sep 18, 2025

View reviewed changes

examples/active_plasma_lens/plot_APL_ChrPlasmaLens_analytical.py Outdated Show resolved Hide resolved

cemitch99 and others added 2 commits September 18, 2025 14:59

Update examples/active_plasma_lens/plot_APL_ChrPlasmaLens_analytical.py

2089acf

Link README to examples documentation.

083d8dd

ax3l reviewed Sep 18, 2025

View reviewed changes

docs/source/usage/examples.rst Outdated Show resolved Hide resolved

cemitch99 reviewed Sep 18, 2025

View reviewed changes

docs/source/usage/examples.rst Outdated Show resolved Hide resolved

Update docs/source/usage/examples.rst

fb81266

ax3l approved these changes Sep 18, 2025

View reviewed changes

cemitch99 reviewed Sep 18, 2025

View reviewed changes

examples/active_plasma_lens/README.rst Outdated Show resolved Hide resolved

cemitch99 reviewed Sep 18, 2025

View reviewed changes

examples/active_plasma_lens/README.rst Outdated Show resolved Hide resolved

Apply suggestions from code review

aa5d51a

cemitch99 enabled auto-merge (squash) September 18, 2025 22:44

cemitch99 merged commit 771000b into BLAST-ImpactX:development Sep 18, 2025
16 checks passed

cemitch99 mentioned this pull request Sep 19, 2025

Gauss3D: Expose Integration Steps #1153

Closed

ax3l deleted the APL-negK branch September 22, 2025 18:01

ChrPlasmaLens element: Support k<0, and fix uninitialized-variable bug in tracking of reference particle #1030

ChrPlasmaLens element: Support k<0, and fix uninitialized-variable bug in tracking of reference particle #1030

Uh oh!

Conversation

kyrsjo commented Jul 4, 2025

Uh oh!

kyrsjo commented Jul 4, 2025

Uh oh!

ax3l commented Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kyrsjo commented Jul 8, 2025

Uh oh!

ax3l commented Jul 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cemitch99 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kyrsjo commented Aug 25, 2025

Uh oh!

cemitch99 Sep 18, 2025

Choose a reason for hiding this comment

Uh oh!

kyrsjo Sep 22, 2025

Choose a reason for hiding this comment

Uh oh!

cemitch99 Sep 18, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ax3l left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kyrsjo commented Sep 22, 2025

Uh oh!

ax3l commented Sep 22, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ax3l commented Jul 7, 2025 •

edited

Loading

ax3l commented Jul 9, 2025 •

edited

Loading