Conversation
|
I've added a benchmark against the KV envelope equations in a drift and FODO lattice: https://github.com/austin-hoover/cheetah/tree/sc2d/tests/test_space_charge_kick_2d. Temporarily putting this in
|
| ), | ||
| ) | ||
|
|
||
| def _deposit_charge_on_grid( |
There was a problem hiding this comment.
could you write general functions for depositing charge on 1/2/3 D grids and put it in a separate? This is something that we want to do for wakefields, images as well
There was a problem hiding this comment.
Not yet. It would be great if you can add this feature.
There was a problem hiding this comment.
Hey Austin, Ryan mentioned this to me in our last meeting. I have code for 1d and 2d grids from our wakefields work which I can send over. I think it may be a good idea to start with 3 separate charge deposition functions, as the 1d case is especially simplified.
There was a problem hiding this comment.
Great! Want to start a new PR with your work?
There was a problem hiding this comment.
Are we wanting to utilize a grid deposition function in this PR, or later on?
There was a problem hiding this comment.
I think it would make sense to first finalize/merge this PR, and then later refactor the code to expose a general grid deposition function that could be used in other parts of the code (incl. in the code that generates beam images).
|
@austin-hoover This looks great, thanks for adding the new feature. |
|
In addition, I think it would be good to have tests for the A key aspect of the CI tests is that they need automated checks (typically with |
|
Okay, I put tests under I didn't include the gradient tests. For a zero-emittance KV beam in free space, the envelope radius |
|
We could compute derivative of single particle |
|
Let me just share what I have via comment, as I'm a bit unclear if we're trying to generalize first or just trying to make cheetah functions for 1d and 2d. These functions copy the steps of the charge deposition function in space charge, but in 1d and 2d. Here's what I have for 1d: def deposit_counts_1d(z): and this is what I have for 2d: |
|
@jank324 I was looking for the charge deposition feature and stumbled on this, are there more fixes to be done for this PR? It's been sitting for quite a while now. Otherwise I would say we should merge it first and work on refactoring the charge deposition function later, as @RemiLehe suggested. |
|
|
||
|
|
||
| class SpaceChargeKick(Element): | ||
| class SpaceChargeKick3D(Element): |
There was a problem hiding this comment.
This is a breaking change. I'm undecided if we should just accept the breaking change or handle it in some way? (e.g. stub class with informative error or something like that). There are pros and cons for both. What are your opinions?
| from cheetah import Segment, SpaceChargeKick2D | ||
|
|
||
|
|
||
| def get_lorentz_factors(rest_energy: float, kin_energy: float) -> tuple[float, float]: |
There was a problem hiding this comment.
There seems to be a huge amount of boilerplate code for the tests here. I would prefer if this mimicked the 3D space charge kick tests, i.e. if it was basically the same but with minor modifications to work for SpaceChargeKick2D.
There was a problem hiding this comment.
At the moment these tests also still fail.
So I really think it would be nicer to match the tests for the 3D element.
There was a problem hiding this comment.
Pull request overview
This PR introduces a new 2D space-charge kick element (SpaceChargeKick2D) for long-bunch / line-charge approximations, and renames the existing space-charge kick implementation to SpaceChargeKick3D to distinguish the solvers.
Changes:
- Added
SpaceChargeKick2Delement (2D integrated Green function solver) and accompanying tests. - Renamed
SpaceChargeKick→SpaceChargeKick3Dand updated exports/usages across the library and tests. - Updated changelog to document the new feature and the breaking rename.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
cheetah/accelerator/space_charge_kick_2d.py |
New 2D space charge kick element implementation. |
cheetah/accelerator/space_charge_kick_3d.py |
Renames the class to SpaceChargeKick3D and minor numeric/style adjustments. |
cheetah/accelerator/__init__.py |
Exports SpaceChargeKick2D and SpaceChargeKick3D from the accelerator package. |
cheetah/__init__.py |
Re-exports SpaceChargeKick2D and SpaceChargeKick3D at top-level cheetah.*. |
tests/test_space_charge_kick_2d.py |
Adds a dedicated 2D space-charge test suite, including an envelope-comparison test. |
tests/test_space_charge_kick_3d.py |
Updates tests to use SpaceChargeKick3D. |
tests/test_vectorized.py |
Updates element coverage list to include both 2D and 3D space-charge elements. |
tests/test_elements.py |
Updates xfail set to include both new space-charge element classes. |
tests/conftest.py |
Updates default element parameterization for both SpaceChargeKick2D and SpaceChargeKick3D. |
CHANGELOG.md |
Documents the breaking rename and new 2D feature. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| beam_length = ( | ||
| (beam.particles[..., 4]).max() - (beam.particles[..., 4]).min() | ||
| ).abs() | ||
| charge_density = ( | ||
| self._array_rho(beam, xp_coordinates, cell_size, grid_dimensions) | ||
| / beam_length | ||
| ) |
There was a problem hiding this comment.
In _solve_poisson_equation, beam_length is computed via a global .max()/.min() over all batches and particles. For vectorized beams this collapses the batch dimension, so every beam instance shares the same beam_length, which will incorrectly scale the charge density (and can vary depending on other batch entries). Compute the longitudinal extent per beam (reduce only over the particle dimension) and broadcast accordingly before dividing.
| beam_length = ( | |
| (beam.particles[..., 4]).max() - (beam.particles[..., 4]).min() | |
| ).abs() | |
| charge_density = ( | |
| self._array_rho(beam, xp_coordinates, cell_size, grid_dimensions) | |
| / beam_length | |
| ) | |
| # Compute longitudinal extent per beam (reduce only over particle dimension) | |
| z_coords = beam.particles[..., 4] | |
| z_max = z_coords.max(dim=-1).values | |
| z_min = z_coords.min(dim=-1).values | |
| beam_length = (z_max - z_min).abs() | |
| charge_density = self._array_rho( | |
| beam, xp_coordinates, cell_size, grid_dimensions | |
| ) | |
| # Broadcast beam_length to match charge_density shape for per-beam scaling | |
| while beam_length.dim() < charge_density.dim(): | |
| beam_length = beam_length.unsqueeze(-1) | |
| charge_density = charge_density / beam_length |
There was a problem hiding this comment.
I don't quite understand why @copilot wants to modify so many more things. I just fixed the vectorisation issue as such ... I think.
| def sample(self, n: int) -> np.ndarray: | ||
| x = np.random.normal(size=(n, 4)) | ||
| x /= np.linalg.norm(x, axis=1)[:, None] | ||
| x /= np.std(x, axis=0) | ||
|
|
||
| S = self.cov() | ||
| L = np.linalg.cholesky(S) | ||
| x = np.matmul(x, L.T) | ||
| return x |
There was a problem hiding this comment.
This test uses NumPy RNG (np.random.normal) but the test suite fixture only seeds Torch RNGs. That makes the test nondeterministic and can cause flaky failures. Please seed NumPy (e.g., via a fixture) or generate the sample using Torch so it is covered by the existing deterministic seeding.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
rho has shape (n, bx, by) and charge_density has shape (n,). So charge_density needs to be reshaped to (n, 1, 1) before dividing.
|
The failure is now coming from /tests/test_elements.py. |
|
Regarding the benchmarks: the overhead is to integrate the KV envelope equations. (You could probably condense it if you wanted to.) The tests in |



Description
This PR adds an element called
SpaceChargeKick2Dwhich applies 2D space charge kicks to the beam. It assumes the beam has an infinite length and uniform density in the longitudinal plane (line charges). The code is pretty much the same as the currentSpaceChargeKickelement. The integrated 2D Green function is taken from https://www.sciencedirect.com/science/article/abs/pii/S0021999104000282?via%3Dihub.I have also renamed
SpaceChargeKicktoSpaceChargeKick3D.Motivation and Context
SpaceChargeCalcelement, the 2D grid is centered at zero and expanded based on the rms beam size, so it may not contain all particles.Types of changes
Checklist
flake8(required).pytesttests pass (required).pyteston a machine with a CUDA GPU and made sure all tests pass (required).Note: We are using a maximum length of 88 characters per line.