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

Space charge 2D envelope push #841

Merged
merged 50 commits into from
Feb 24, 2025

Conversation

cemitch99
Copy link
Member

@cemitch99 cemitch99 commented Feb 11, 2025

This PR implements the linear push of the covariance matrix in the presence of 2D space charge.

  • Basic math for space charge push
  • Add calculation of beam perveance
  • Add beam current user input
  • Update space charge flag to allow 2D/3D options
  • Adjust location to src/envelope
  • Add Python bindings
  • Benchmark example
  • Update documentation
  • Finalize control parameters & update examples

This addresses Issue #826 in the 2D (envelope) case. Follow-up: a corresponding model for 3D space charge.

@ax3l ax3l self-requested a review February 11, 2025 00:35
@ax3l ax3l added the component: space charge Space charge & potential solver label Feb 11, 2025
cemitch99 and others added 8 commits February 11, 2025 10:01
updates:
- [github.com/astral-sh/ruff-pre-commit: v0.9.4 → v0.9.6](astral-sh/ruff-pre-commit@v0.9.4...v0.9.6)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Move "workflows" to "how-to guides" as in WarpX.
Clean up some wording.
Currently, we only support (and document correctly) "static" units,
but the user was able to also pass "dynamic", upon which we did print
"Dynamic units" but then still used static ones.

This now errors out correctly for anything else than "static" until
we implement another variant.
* `Source` Element for openPMD

* Rename Source Files of `BeamMonitor`

* Deduplicate openPMD Logic Helpers

* Docs

* Example

* Doc: Fix Copy-Paste
* update dashboard __init__.py import
* Add tooltips functionality to non-nested states
* Add tooltips for the custom vselects
* update type hints
Comment on lines 314 to 323
/*
using SrcData = WarpXParticleContainer::ParticleTileType::ConstParticleTileDataType;
tmp.copyParticles(*pc,
[=] AMREX_GPU_HOST_DEVICE (const SrcData& src, int ip, const amrex::RandomEngine& engine)
{
const SuperParticleType& p = src.getSuperParticle(ip);
return random_filter(p, engine) * uniform_filter(p, engine)
* parser_filter(p, engine) * geometry_filter(p, engine);
}, true);
*/

Check notice

Code scanning / CodeQL

Commented-out code Note

This comment appears to contain commented-out code.
Comment on lines 405 to 415
/*
// comment this in once IntSoA::nattribs is > 0

std::copy(IntSoA::names_s.begin(), IntSoA::names_s.end(), int_soa_names.begin());

for (auto int_idx=0; int_idx < RealSoA::nattribs; int_idx++) {
auto const component_name = int_soa_names.at(int_idx);
getComponentRecord(component_name).storeChunkRaw(
soa.GetIntData(int_idx).data(), {offset}, {numParticleOnTile64});
}
*/

Check notice

Code scanning / CodeQL

Commented-out code Note

This comment appears to contain commented-out code.
Copy link
Member

@ax3l ax3l 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 awesome!

Added small inline comments.

@ax3l ax3l self-assigned this Feb 20, 2025
Comment on lines 483 to 490
if (space_charge_model == "3D") {
pp_dist.query("charge", intensity);
throw std::runtime_error("3D space charge model not yet implemented in envelope mode.");
} else if (space_charge_model == "2D") {
pp_dist.query("current", intensity);
} else {
throw std::runtime_error("Unknown space_charge_model (use '2D' or '3D') ");
}
Copy link
Member

Choose a reason for hiding this comment

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

I think I want to unify this with algo.space_charge. There is no need for two parameters, we can just do algo.space_charge = false (default), = 2D and = 3D.

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 makes sense, although it will break the existing space charge examples, where algo.space_charge = true. In that case, maybe we should push the change through as a follow-up, since it also impacts the logic outside the envelope tracking?

Copy link
Member

Choose a reason for hiding this comment

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

I think I can implement it in a backwards compatible way, will give it a try tomorrow.

Copy link
Member

@ax3l ax3l Feb 21, 2025

Choose a reason for hiding this comment

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

I pushed a backwards compatible change and it currently passes CI. This is what existing users will see (True defaults now to 3D space charge and raises a warning to use the new syntax.)

I will push another commit now that updates all examples to the new syntax, so people see and use that going forward. After a few months (year), we can drop support for the old syntax. I added an inline TODO where we will remove it.

@ax3l ax3l force-pushed the add_spch2D_envelope branch 2 times, most recently from 04dd198 to e45cafd Compare February 21, 2025 08:24
Comment on lines +487 to +488
//pp_dist.get("charge", intensity);
//amr_data->track_envelope.m_env = impactx::initialization::create_envelope(dist, intensity);

Check notice

Code scanning / CodeQL

Commented-out code Note

This comment appears to contain commented-out code.
@@ -40,7 +40,7 @@ collimator.aperture_y = 1.5e-3
# Algorithms
###############################################################################
algo.particle_shape = 2
algo.space_charge = false
Copy link
Member Author

Choose a reason for hiding this comment

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

I have one concern about this recent change. With algo.space_charge = false, it is obvious to the user that space charge is not active. Without this line, I think it is too easy to forget that space charge is off by default, especially when a nonzero bunch charge is provided. This is especially important in the examples. If we keep three string input options "2D", "3D", and "false", then we could keep this line.

Copy link
Member

Choose a reason for hiding this comment

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

I was wondering about that, good point.

Off was and is the default; I agree we can keep it explicit to avoid any confusion.

pp_algo.add("space_charge", std::string("3D"));
} else {
py::print("sim.space_charge = False is deprecated, please use space_charge = \"off\" (or skip for default)");
pp_algo.add("space_charge", std::string("off"));
Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I see that the "off" string is supported, so I would keep this in the example input files.

Copy link
Member

Choose a reason for hiding this comment

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

Agreed. And we can still use false as well.

The problem why I need to internally call this off in the enum is that false is a C++ keyword.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, can name it False internally (uppercase)

@ax3l ax3l force-pushed the add_spch2D_envelope branch 2 times, most recently from 3acf310 to 716c149 Compare February 21, 2025 19:19
@ax3l ax3l force-pushed the add_spch2D_envelope branch from 716c149 to 6e65816 Compare February 21, 2025 19:26
ax3l added 2 commits February 24, 2025 10:08
And add doxygen and improve case matching.
@ax3l ax3l enabled auto-merge (squash) February 24, 2025 18:26
@ax3l ax3l merged commit e670d0e into BLAST-ImpactX:development Feb 24, 2025
15 of 16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: space charge Space charge & potential solver
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants