Modular stellar-evolution framework for Star particles#1962
Conversation
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…model Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…etadata Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
… descriptor Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…pdate path Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…dation problem Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…lidation test Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ce DefaultParticleTraits Replace standalone StellarModel_Traits with a stellar_model alias in DefaultParticleTraits, inherited by all Particle_Traits specializations. This avoids updating every problem when new particle-trait parameters are added. All 15 existing test problems now inherit from DefaultParticleTraits. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- Shrink r_B to ~0.1*dx (M0=0.1 Msun, half_size=1.2e19) so accretion is in the sub-grid Bondi regime. - Add compile-time assertion that r_B < 0.1*dx. - Reduce B0 to 1e-11 so that beta >> 1 and cf ~ cs, making the numerical Bondi rate match the unmagnetized analytic formula. - Fix the one-step lag in the luminosity check: the stellar update runs before accretion, so pair each recorded L[i] with the prior step's mdot. - Assert that the mean numerical dM/dt matches the analytic Bondi rate within 10%. - Add createInitialStarParticles to AdvectionSimulation. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
for more information, see https://pre-commit.ci
StellarUpdate is only declared in starparticle_radiation.hpp (guarded by AMREX_SPACEDIM==3). The Star ParticlePropertyUpdateTraits specialization must be similarly guarded, otherwise 1D/2D builds fail with "StellarUpdate has not been declared". Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- Add mass <= 0.0 guards in radius() and luminosityStar() to prevent std::pow domain errors with fractional exponents. - Update docs to reference Particle_Traits<problem_t>::stellar_model instead of the removed StellarModel_Traits. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Replace the manual particle-tile creation with InitFromAsciiFile, which is MPI-safe (only rank 0 reads the file) and avoids duplicate particles on multi-rank runs. The position is then adjusted to the cell centre in a post-processing pass, matching the ParticleAccretion pattern. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
M0_in_Msun is captured in a GPU ParallelFor lambda and must be accessible from device code. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- Guard StarParticleLumIdx write with if constexpr (Nout > 0) to prevent out-of-bounds access when radiation is disabled. - Zero-fill remaining luminosity groups when Nout > 1 so that uninitialised values aren't deposited into multigroup radiation. - Pass -1 for lum_idx to the Star descriptor when nGroups == 0. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
for more information, see https://pre-commit.ci
When restarting from a checkpoint, Star particles were always freshly created via createInitialStarParticles(), discarding the checkpointed mass/mdot/radius/luminosity state. Match the pattern used by all other particle types and call initializeParticleContainerFromCheckpoint when is_restart is true. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Broaden Model::evolve() from evolve(mass, mdot, dt, radius_out, lum_out) to evolve(mass&, mdot, radius_val&, lum*, n_groups, dt). This lets each model decide what to read and modify: - mass and radius are in/out (model may read current values, e.g. for stateful integration, or modify them, e.g. wind mass loss) - lum is a pointer to all n_groups luminosity slots (model writes all groups, supporting multi-group radiation) The dispatcher in StellarUpdate reads current mass/mdot/radius from the particle, passes them to evolve(), and writes back any changes. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Extract StarParticleDataIdx into a standalone star_particle_indices.H so stellar models can include the index constants without pulling in the full particle type machinery (avoiding circular includes). Model::evolve() now takes (Real *rdata, int n_groups, Real dt) — the complete particle real-data array — letting each model read and modify any component (mass, mdot, radius, lum, velocity, etc.) using the named indices. The dispatcher in StellarUpdate simplifies to a single call: Model::evolve(&p.rdata(0), Nout, dt). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request introduces a new Star particle type to support modular, pluggable stellar-evolution models selected at compile time, along with a stateless ToyStellarModel and a validation test problem (ParticleStarEvolution). The reviewer identified a critical issue in the validation test's computeAfterTimestep function, where particle data is recorded only on the IO processor (rank 0). In a parallel MPI simulation, if the single star particle resides on a different rank, the IO processor will have empty data, causing the validation checks to fail. The reviewer provided a code suggestion to resolve this by extracting the particle properties on the owning rank and performing a global sum reduction across all ranks.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Replace `const auto &real_data = ...getParticleDataAtLevel(...).first` with structured binding `const auto [real_data, int_data] = ...` to avoid binding a reference to a member of a temporary pair. This matches the pattern used in BinaryOrbitCIC and is MPI-safe across all ranks. Also iterate with range-for instead of accessing real_data[0] directly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
for more information, see https://pre-commit.ci
Two fixes: 1. Add static_assert preventing both ParticleSwitch::Sink and ParticleSwitch::Star from being enabled simultaneously. Both accrete via the same accretion-rate buffer, and the current computeSinkAccretion / applySinkAccretion dispatch would double- apply gas removal. Includes a comment describing the refactoring needed to support multiple accreting types in the future. 2. Pass integer particle data (p.idata()) to Model::evolve() so that stellar models with nExtraInt > 0 (e.g. ORION) can read and update integer state. The toy model ignores it; the dispatcher passes nullptr when NInt == 0. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
for more information, see https://pre-commit.ci
|
@markkrumholz Ready for review again. |
markkrumholz
left a comment
There was a problem hiding this comment.
There's a merge conflict that needs to be resolved, but on top of that, I'm confused by the design of passing an array of gpu_constants data to the evolve routine. Why do that? Why not just have the evolve routine implemented by each star particle class have access to the internal class members, some of which can be data tables that live on GPU? Since these tables are presumably read once during startup and then stored and not altered, I don't see the reason to pass them around rather than just have them be class members that are set on initialization.
|
@chongchonghe, comments added. |
…ultPhysicsTraits Address Mark's review comments: 1. Add StarParticleIntIdx enum (stage) and StarParticleIntegerComps alias, wire expandEnumNames into getParticleIntCompNames for Star particles. When nExtraInt > 0 (ORION model), names expand as stage_0, stage_1, ... 2. Remove gpu_tables from the Star stellar-evolution update path. Stellar models own their internal tables. The updateParticleProperties specialization now inlines its own loop instead of calling applyUpdate with a dummy LuminosityGpuConstTables. 3. Inherit Physics_Traits<StarEvolutionProblem> from DefaultPhysicsTraits to pick up the resistity_model default. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Remove gpu_tables parameter from the entire update chain: - applyUpdate() no longer takes gpu_tables - updateProperties() no longer takes gpu_tables - updateLuminosity() no longer takes gpu_tables — it reads g_device_luminosity_tables<Nout> directly (AMREX_GPU_MANAGED) StochasticStellarPop::updateParticleProperties loads tables into the global before calling applyUpdate. Star::updateProperties inherits updateParticleProperties from the base class (no custom loop needed). All particle types now share the same applyUpdate signature. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Without this, Particle_Traits<SubcycleProblem> has no stellar_model typedef, causing StarParticleRealComps<SubcycleProblem> to fail on CUDA when std::unique_ptr<StarParticleContainer<SubcycleProblem>> requires the complete type (AMRSimulation member for all problem types). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
/azp run quick |
|
No pipelines are associated with this pull request. |
|
/azp run quick |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@markkrumholz All comments addressed. Ready for review. |
|
/azp run rocm-quick |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Description
Add a modular, compile-time-selectable stellar-evolution framework for
Starparticles, with a stateless analytic toy model as the default, plus a validation test that checks radius and luminosity against closed-form solutions and asserts the numerical Bondi accretion rate.Motivation
Currently Quokka has no
Starparticle type with stellar-evolution physics. This PR adds the framework, a toy model, and a validating test problem so that more sophisticated stateful models (e.g., ORION model or tabulated stellar tracks) can be plugged in later without changing the dispatch machinery.What changed
Framework (
src/particles/stellar_models.hpp,src/particles/starparticle_radiation.hpp)ToyStellarModel— stateless analytic laws: R ∝ M^0.4, L_star ∝ M^3.5, L_acc = GMṁ/R. Pure GPU device functions.Model::evolve(Real *rdata, int n_groups, Real dt)— takes the full particle real-data array so the model can read and modify any component (mass, radius, luminosity groups, velocity, etc.). The dispatcher simply passes&p.rdata(0)— the model owns all field access.Shared index header (
src/particles/star_particle_indices.H— new)StarParticleDataIdxenum and all index constants (StarParticleMassIdx,StarParticleMdotIdx,StarParticleRadiusIdx,StarParticleLumIdx, etc.) into a standalone header that bothstellar_models.hppandparticle_types.hppinclude. Avoids circular dependencies while letting models use named indices.Star particle type (
src/particles/particle_types.hpp,src/particles/PhysicsParticles.hpp)ParticleSwitch::StarandParticleType::Star.mass, vx, vy, vz, birth_time, mdot, radius, lumwithlumexpandable tonGroupsslots. Component count is auto-composed fromnGroups+ the model'snExtraReal/nExtraInt.allows_accretion=trueandmdotIndex_=StarParticleMdotIdx.DefaultParticleTraitsrefactoring (particle_types.hpp+ 15 test problems)DefaultParticleTraitsas a base struct holdingparticle_switch = Noneandusing stellar_model = quokka::ToyStellarModel.Particle_Traits<problem_t>now inherits from it. This avoids having to touch every test problem when new particle-trait parameters are added.Particle_Traitsspecializations updated to: DefaultParticleTraits.StellarModel_Traits— model selection now goes throughParticle_Traits::stellar_model.Simulation wiring (
src/simulation.hpp,src/QuokkaSimulation.hpp,src/linear_advection/AdvectionSimulation.hpp)StarParticlescontainer member,createInitialStarParticles()pure virtual + default empty definition, Star block inInitPhyParticles.initializeParticleContainerFromCheckpoint) matching the pattern used by all other particle types.Validation test (
src/problems/ParticleStarEvolution/)Documentation (
docs/markdown/particles.md)Related issues
N/A
Checklist
Before this pull request can be reviewed, all of these tasks should be completed. Denote completed tasks with an
xinside the square brackets[ ]in the Markdown source below:/azp run.