Skip to content

Polymorphic injectors, external currents, and diagnostics #377

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.

Sign up for GitHub

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

Merged
JamesMcClung merged 29 commits into from
Feb 20, 2026
Merged

Polymorphic injectors, external currents, and diagnostics #377

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
f62ec7c
+injector_base: InjectorBase, InjectorFromLambda
JamesMcClung Jan 23, 2026
496ad5c
boundary_injector: inherit from InjectorBase
JamesMcClung Jan 23, 2026
7a35b95
psc: have a list of InjectorBase ptrs
JamesMcClung Jan 23, 2026
a175f13
*: add injectors to the list
JamesMcClung Jan 23, 2026
99beaf1
psc; *: rm templated injector
JamesMcClung Jan 23, 2026
1be2d52
psc: -InjectParticlesNone
JamesMcClung Jan 23, 2026
1c2ddfb
-composite_injector; *
JamesMcClung Jan 23, 2026
3931fad
+external_current_base
JamesMcClung Jan 23, 2026
0e6f2a5
psc; *: use vec of ExternalCurrents
JamesMcClung Jan 23, 2026
ddc197a
psc: -ExternalCurrentNone
JamesMcClung Jan 23, 2026
bc43fb1
psc; *: make vecs private
JamesMcClung Jan 23, 2026
358a042
boundary_injector: don't impl op()
JamesMcClung Jan 23, 2026
c3fbc68
psc; *: initialize -> pre_first_step
JamesMcClung Jan 23, 2026
f4a58a6
+diagnostic_base
JamesMcClung Jan 27, 2026
7ee609c
OutputFieldsDefault: make diagnostic
JamesMcClung Jan 27, 2026
dceac22
output_particles*; *: make diagnostic
JamesMcClung Jan 27, 2026
ad80822
DiagEnergies; *: make diagnostic
JamesMcClung Jan 27, 2026
95256de
psc; *: abstract diagnostics
JamesMcClung Jan 27, 2026
77fa1a5
psc: update stat in perform_diagnostics
JamesMcClung Jan 27, 2026
119a734
-DiagnosticsDefault; *
JamesMcClung Jan 27, 2026
17fe7ab
tests: don't make diagnostics
JamesMcClung Jan 27, 2026
70c54da
psc: fix capture by ref -> segfault
JamesMcClung Feb 5, 2026
bb1298b
injector_base: lambda_
JamesMcClung Feb 10, 2026
dc5cf86
*_base: more lambda_
JamesMcClung Feb 10, 2026
3c16414
psc: assert pointers
JamesMcClung Feb 10, 2026
79e444a
_*base: include functional
JamesMcClung Feb 11, 2026
9707ee8
*_base: add virtual dtors
JamesMcClung Feb 18, 2026
88e83fe
*_base: don't use templated types
JamesMcClung Feb 18, 2026
02902d0
psc: solve the problem with a todo
JamesMcClung Feb 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions src/include/boundary_injector.hxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "setup_particles.hxx"
#include "kg/VecRange.hxx"
#include "../libpsc/psc_push_particles/inc_push.cxx"
#include "injector_base.hxx"

/// @brief A particle generator for use with @ref BoundaryInjector. Samples
/// particles from a (possibly shifted) Maxwellian distribution.
Expand Down Expand Up @@ -62,6 +63,8 @@ private:
/// `MfieldsState`, `Current`, `real_t`, etc.
template <typename PARTICLE_GENERATOR, typename PUSH_PARTICLES>
class BoundaryInjector
: public InjectorBase<typename PUSH_PARTICLES::Mparticles,
typename PUSH_PARTICLES::MfieldsState>
{
static const int INJECT_DIM_IDX_ = 1;

Expand All @@ -82,6 +85,11 @@ public:
prts_per_unit_density_{grid.norm.prts_per_unit_density}
{}

void inject(Mparticles& mprts, MfieldsState& mflds) override
{
(*this)(mprts, mflds);
}

Copy link
Copy Markdown
Contributor

@germasch germasch Feb 9, 2026

Choose a reason for hiding this comment

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

I may learn more as I go through more commits, but at this point I'm wondering, why not stick with operator()?

Copy link
Copy Markdown
Collaborator Author

@JamesMcClung JamesMcClung Feb 10, 2026

Choose a reason for hiding this comment

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

I would say explicit method names helps with discoverability: one can search for implementations of named methods, but not for operator(). That's also a benefit of using polymorphism rather than templates or outright type erasure (à la std::function).

Copy link
Copy Markdown
Collaborator Author

@JamesMcClung JamesMcClung Feb 10, 2026

Choose a reason for hiding this comment

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

Following my own logic, I should get rid of the lambda wrappers and force users to make their own subclasses. That would be better future-proofed against more methods, too (such as requiring subclasses to provide logging-related information).

Copy link
Copy Markdown
Contributor

@germasch germasch Feb 13, 2026

Choose a reason for hiding this comment

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

I guess I'm not entirely clear on my own thinking here, though I think it's nice isolating users from having to do their own subclasses -- though admittedly lambda functions are likely an even more unfamiliar feature to a casual user...

Anyway, I guess what's there currently (class from lambda) is fine for the time being.

/// Injects particles at the lower y-bound as if there were a population of
/// particles just beyond the edge. The imaginary particle population has unit
/// density, and individual particles from that population are sampled using
Expand Down
29 changes: 29 additions & 0 deletions src/include/injector_base.hxx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
#pragma once

template <typename MPARTICLES, typename MFIELDS_STATE>
struct InjectorBase
{
using Mparticles = MPARTICLES;
using MfieldsState = MFIELDS_STATE;

virtual void inject(Mparticles& mprts, MfieldsState& mflds) = 0;
};

template <typename MPARTICLES, typename MFIELDS_STATE>
struct InjectFromLambda : InjectorBase<MPARTICLES, MFIELDS_STATE>
{
using Mparticles = MPARTICLES;
using MfieldsState = MFIELDS_STATE;

InjectFromLambda(std::function<void(Mparticles&, MfieldsState&)> lambda)
: lambda{lambda}
{}

void inject(Mparticles& mprts, MfieldsState& mflds) override
{
return lambda(mprts, mflds);
}

private:
std::function<void(Mparticles&, MfieldsState&)> lambda;
Comment thread
JamesMcClung marked this conversation as resolved.
Outdated
};
7 changes: 6 additions & 1 deletion src/include/psc.hxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <setup_particles.hxx>

#include "../libpsc/vpic/fields_item_vpic.hxx"
#include "injector_base.hxx"
#include <checks_params.hxx>
#include <output_particles.hxx>
#include <push_particles.hxx>
Expand Down Expand Up @@ -343,7 +344,9 @@ struct Psc

// === particle injection
prof_start(pr_inject_prts);
inject_particles_(mprts_, mflds_);
for (auto injector : injectors) {
injector->inject(mprts_, mflds_);
}
prof_stop(pr_inject_prts);

// === external current
Expand Down Expand Up @@ -470,6 +473,8 @@ private:
public:
const Grid_t& grid() { return *grid_; }

std::vector<InjectorBase<Mparticles, MfieldsState>*> injectors;

Copy link
Copy Markdown
Contributor

@germasch germasch Feb 9, 2026

Choose a reason for hiding this comment

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

I think I read somewhere that ideally modern C++ shouldn't use explicit pointers at all. The question here is, what guarantees that the objects pointed to don't go away?

Copy link
Copy Markdown
Collaborator Author

@JamesMcClung JamesMcClung Feb 10, 2026

Choose a reason for hiding this comment

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

I kinda felt like using fancy pointer wrappers would be more performative than actually beneficial, since the psc integrator may as well be a singleton, and an entire case revolves around calling integrate() once. Thus the leaking :^)
But I'll go back and do the right thing...

Copy link
Copy Markdown
Collaborator Author

@JamesMcClung JamesMcClung Feb 10, 2026

Choose a reason for hiding this comment

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

Actually, what would be the best option, if not unique_ptr?

Copy link
Copy Markdown
Collaborator Author

@JamesMcClung JamesMcClung Feb 13, 2026

Choose a reason for hiding this comment

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

As I'm working on switching to unique_ptrs, I'm finding it annoying to have to dynamically allocate all the components. I'm leaning back towards keeping raw pointers, since it's nicer to just stack-allocate things.

Copy link
Copy Markdown
Contributor

@germasch germasch Feb 13, 2026

Choose a reason for hiding this comment

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

I guess I haven't looked at the various places where this is being used, the ones I saw initially had a new anyway. I think transferring ownership is cleaner. I suppose one could have add_injector() or whatever dynamically copy construct the thing, which would hide the new, but at the expense of needing the injectors to be copy-constructible.

The other alternative I can think of is to just keep a reference, ie., punt on the ownership question -- the one thing that's nice about that is that it guarantees non-null, but generally speaking, I think clear ownership rules are preferable. And I can kinda think of test cases where one might want to have to integrators going at the same time for some kind of cross checking, where it'd generally be nice to not assume singleton behavior, though it's kinda far fetched.

Copy link
Copy Markdown
Collaborator Author

@JamesMcClung JamesMcClung Feb 17, 2026

Choose a reason for hiding this comment

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

The news were there to avoid having to stack-allocate on a separate line, which references can't do. I realize that isn't a good justification for raw pointers. I'll try to find a better solution that achieves clear ownership without introducing excessive boilerplate.

private:
double time_start_;
PscParams p_;
Expand Down
33 changes: 17 additions & 16 deletions src/libpsc/tests/test_boundary_injector.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,13 +142,13 @@ TEST(BoundaryInjectorTest, Integration1Particle)
DiagEnergies oute{grid.comm(), 0};
auto diagnostics = makeDiagnosticsDefault(outf, outp, oute);

auto inject_particles =
BoundaryInjector<ParticleGenerator, PscConfig::PushParticles>{
ParticleGenerator(1, 1), grid};
auto psc =
makePscIntegrator<PscConfig>(psc_params, grid, mflds, mprts, balance,
collision, checks, marder, diagnostics);

auto psc = makePscIntegrator<PscConfig>(psc_params, grid, mflds, mprts,
balance, collision, checks, marder,
diagnostics, inject_particles);
psc.injectors.push_back(
new BoundaryInjector<ParticleGenerator, typename PscConfig::PushParticles>(
ParticleGenerator(1, 1), grid));
Copy link
Copy Markdown
Contributor

@germasch germasch Feb 9, 2026

Choose a reason for hiding this comment

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

I guess this answers the question about the injectors not going away, essentially because they're being leaked ;) Obviously not a big issue as such, but I think unique_ptr may be the better option here.

Another thing I don't fully like is that the injectors are added after the fact, which kinda means that the integrator isn't fully set up after being constructed. But I think it's acceptable here, in that an empty list of integrators is a valid state.

Copy link
Copy Markdown
Contributor

@germasch germasch Feb 9, 2026

Choose a reason for hiding this comment

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

And I would have to agree that, as we currently do, passing N objects to the constructor makes that rather ugly...

Copy link
Copy Markdown
Collaborator Author

@JamesMcClung JamesMcClung Feb 10, 2026

Choose a reason for hiding this comment

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

The partially-constructed state of the integrator makes me uncomfortable, too... and I was only planning to make it worse over the next few weeks. This seems like a perfect place to use the builder pattern, but it might be a while before that end state is reached.


// ----------------------------------------------------------------------
// set up initial conditions
Expand Down Expand Up @@ -205,13 +205,13 @@ TEST(BoundaryInjectorTest, IntegrationManyParticles)
DiagEnergies oute{grid.comm(), 0};
auto diagnostics = makeDiagnosticsDefault(outf, outp, oute);

auto inject_particles =
BoundaryInjector<ParticleGenerator, PscConfig::PushParticles>{
ParticleGenerator(-1, 1), grid};
auto psc =
makePscIntegrator<PscConfig>(psc_params, grid, mflds, mprts, balance,
collision, checks, marder, diagnostics);

auto psc = makePscIntegrator<PscConfig>(psc_params, grid, mflds, mprts,
balance, collision, checks, marder,
diagnostics, inject_particles);
psc.injectors.push_back(
new BoundaryInjector<ParticleGenerator, PscConfig::PushParticles>(
ParticleGenerator(-1, 1), grid));

// ----------------------------------------------------------------------
// set up initial conditions
Expand Down Expand Up @@ -275,11 +275,12 @@ TEST(BoundaryInjectorTest, IntegrationManySpecies)
BoundaryInjector<ParticleGenerator, PscConfig::PushParticles>{
ParticleGenerator(-1, 1), grid};

auto inject = make_composite(inject_electrons, inject_ions);
auto psc =
makePscIntegrator<PscConfig>(psc_params, grid, mflds, mprts, balance,
collision, checks, marder, diagnostics);

auto psc = makePscIntegrator<PscConfig>(psc_params, grid, mflds, mprts,
balance, collision, checks, marder,
diagnostics, inject);
psc.injectors.push_back(&inject_ions);
psc.injectors.push_back(&inject_electrons);

// ----------------------------------------------------------------------
// set up initial conditions
Expand Down
9 changes: 6 additions & 3 deletions src/psc_2d_shock.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -608,9 +608,12 @@ void run()
// ----------------------------------------------------------------------
// hand off to PscIntegrator to run the simulation

auto psc = makePscIntegrator<PscConfig>(psc_params, *grid_ptr, mflds, mprts,
balance, collision, checks, marder,
diagnostics, lf_inject_heat);
auto psc =
makePscIntegrator<PscConfig>(psc_params, *grid_ptr, mflds, mprts, balance,
collision, checks, marder, diagnostics);

psc.injectors.push_back(
new InjectFromLambda<Mparticles, MfieldsState>(lf_inject_heat));

psc.integrate();
}
Expand Down
9 changes: 6 additions & 3 deletions src/psc_flatfoil_yz.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -679,9 +679,12 @@ void run()
// ----------------------------------------------------------------------
// hand off to PscIntegrator to run the simulation

auto psc = makePscIntegrator<PscConfig>(psc_params, *grid_ptr, mflds, mprts,
balance, collision, checks, marder,
diagnostics, lf_inject_heat);
auto psc =
makePscIntegrator<PscConfig>(psc_params, *grid_ptr, mflds, mprts, balance,
collision, checks, marder, diagnostics);

psc.injectors.push_back(
new InjectFromLambda<Mparticles, MfieldsState>(lf_inject_heat));

MEM_STATS();
psc.integrate();
Expand Down