From 038884c4efa4d0ac918ddc30a1801364b3a7b59c Mon Sep 17 00:00:00 2001 From: Seth R Johnson Date: Thu, 23 Jan 2025 13:14:38 -0800 Subject: [PATCH 1/7] Add an optional parameter for setting state size via params --- app/celer-sim/Runner.cc | 10 ++++++---- app/celer-sim/Transporter.cc | 1 - app/celer-sim/Transporter.hh | 4 +--- src/accel/LocalTransporter.cc | 1 - src/accel/SharedParams.cc | 3 +++ src/celeritas/global/CoreParams.hh | 9 +++++++++ src/celeritas/global/CoreState.cc | 10 ++++++++++ src/celeritas/global/CoreState.hh | 3 +++ src/celeritas/global/Stepper.cc | 12 +++++++++++- src/celeritas/global/Stepper.hh | 6 ++---- 10 files changed, 45 insertions(+), 14 deletions(-) diff --git a/app/celer-sim/Runner.cc b/app/celer-sim/Runner.cc index a9bcc66b39..d9510375e4 100644 --- a/app/celer-sim/Runner.cc +++ b/app/celer-sim/Runner.cc @@ -448,6 +448,12 @@ void Runner::build_core_params(RunnerInput const& inp, "streams (" << params.max_streams << " requested)"); + // Store number of tracks per stream + CELER_VALIDATE(inp.num_track_slots > 0, + << "nonpositive num_track_slots=" << inp.num_track_slots); + params.tracks_per_stream + = ceil_div(inp.num_track_slots, params.max_streams); + // Construct track initialization params params.init = [&inp, ¶ms, num_events] { CELER_VALIDATE(inp.initializer_capacity > 0, @@ -469,14 +475,10 @@ void Runner::build_core_params(RunnerInput const& inp, */ void Runner::build_transporter_input(RunnerInput const& inp) { - CELER_VALIDATE(inp.num_track_slots > 0, - << "nonpositive num_track_slots=" << inp.num_track_slots); CELER_VALIDATE(inp.max_steps > 0, << "nonpositive max_steps=" << inp.max_steps); transporter_input_ = std::make_shared(); - transporter_input_->num_track_slots - = ceil_div(inp.num_track_slots, core_params_->max_streams()); transporter_input_->max_steps = inp.max_steps; transporter_input_->store_track_counts = inp.write_track_counts; transporter_input_->store_step_times = inp.write_step_times; diff --git a/app/celer-sim/Transporter.cc b/app/celer-sim/Transporter.cc index 37689a772b..c3c3eba7c3 100644 --- a/app/celer-sim/Transporter.cc +++ b/app/celer-sim/Transporter.cc @@ -72,7 +72,6 @@ Transporter::Transporter(TransporterInput inp) CELER_LOG_LOCAL(status) << "Creating states"; StepperInput step_input; step_input.params = inp.params; - step_input.num_track_slots = inp.num_track_slots; step_input.stream_id = inp.stream_id; step_input.action_times = inp.action_times; stepper_ = std::make_shared>(std::move(step_input)); diff --git a/app/celer-sim/Transporter.hh b/app/celer-sim/Transporter.hh index bf65f3e780..f2c4d6927b 100644 --- a/app/celer-sim/Transporter.hh +++ b/app/celer-sim/Transporter.hh @@ -36,7 +36,6 @@ struct TransporterInput { // Stepper input std::shared_ptr params; - size_type num_track_slots{}; //!< AKA max_num_tracks bool action_times{false}; //!< Whether to synchronize device between //!< actions for timing @@ -51,8 +50,7 @@ struct TransporterInput //! True if all params are assigned explicit operator bool() const { - return params && num_track_slots > 0 && max_steps > 0 - && log_progress > 0; + return params && max_steps > 0 && log_progress > 0; } }; diff --git a/src/accel/LocalTransporter.cc b/src/accel/LocalTransporter.cc index 7bb6e3974c..4e0b68dcbe 100644 --- a/src/accel/LocalTransporter.cc +++ b/src/accel/LocalTransporter.cc @@ -143,7 +143,6 @@ LocalTransporter::LocalTransporter(SetupOptions const& options, StepperInput inp; inp.params = params.Params(); inp.stream_id = stream_id; - inp.num_track_slots = options.max_num_tracks; inp.action_times = options.action_times; if (celeritas::device()) diff --git a/src/accel/SharedParams.cc b/src/accel/SharedParams.cc index cfd00d7321..d3adbe77fe 100644 --- a/src/accel/SharedParams.cc +++ b/src/accel/SharedParams.cc @@ -601,6 +601,9 @@ void SharedParams::initialize_core(SetupOptions const& options) params.max_streams = this->num_streams(); } + // Set state size + params.tracks_per_stream = options.max_num_tracks; + // Allocate device streams, or use the default stream if there is only one. if (celeritas::device() && !options.default_stream && params.max_streams > 1) diff --git a/src/celeritas/global/CoreParams.hh b/src/celeritas/global/CoreParams.hh index ddbb615918..a72047143b 100644 --- a/src/celeritas/global/CoreParams.hh +++ b/src/celeritas/global/CoreParams.hh @@ -37,6 +37,9 @@ class WentzelOKVIParams; //---------------------------------------------------------------------------// /*! * Global parameters required to run a problem. + * + * \todo Always \c tracks_per_stream to build actual states. Currently unit + * tests build states independently. */ class CoreParams final : public ParamsDataInterface { @@ -85,6 +88,9 @@ class CoreParams final : public ParamsDataInterface //! Maximum number of simultaneous threads/tasks per process StreamId::size_type max_streams{1}; + //! Number of track slots per stream + StreamId::size_type tracks_per_stream{0}; + //! True if all params are assigned and valid explicit operator bool() const { @@ -135,6 +141,9 @@ class CoreParams final : public ParamsDataInterface //! Maximum number of streams size_type max_streams() const { return input_.max_streams; } + //! Number of track slots per stream + size_type tracks_per_stream() const { return input_.tracks_per_stream; } + private: Input input_; HostRef host_ref_; diff --git a/src/celeritas/global/CoreState.cc b/src/celeritas/global/CoreState.cc index e2e996baf2..0d7d0fabff 100644 --- a/src/celeritas/global/CoreState.cc +++ b/src/celeritas/global/CoreState.cc @@ -19,6 +19,16 @@ namespace celeritas //! Support polymorphic deletion CoreStateInterface::~CoreStateInterface() = default; +//---------------------------------------------------------------------------// +/*! + * Construct from CoreParams. + */ +template +CoreState::CoreState(CoreParams const& params, StreamId stream_id) + : CoreState{params, stream_id, params.tracks_per_stream()} +{ +} + //---------------------------------------------------------------------------// /*! * Construct from CoreParams. diff --git a/src/celeritas/global/CoreState.hh b/src/celeritas/global/CoreState.hh index 132dee5d7a..298b3414b8 100644 --- a/src/celeritas/global/CoreState.hh +++ b/src/celeritas/global/CoreState.hh @@ -84,6 +84,9 @@ class CoreState final : public CoreStateInterface public: // Construct from CoreParams + CoreState(CoreParams const& params, StreamId stream_id); + + // Construct witih manual slot count CoreState(CoreParams const& params, StreamId stream_id, size_type num_track_slots); diff --git a/src/celeritas/global/Stepper.cc b/src/celeritas/global/Stepper.cc index 60f7624f33..067e897a1d 100644 --- a/src/celeritas/global/Stepper.cc +++ b/src/celeritas/global/Stepper.cc @@ -69,6 +69,10 @@ Stepper::Stepper(Input input) return std::make_shared(*params_->action_reg(), opts); }()} { + // NOTE: if this fails, you're probably running from a unit test? + CELER_VALIDATE(params_->tracks_per_stream() > 0, + << "core params `tracks_per_stream` was not set"); + // Save primary action: TODO this is a hack and should be refactored so // that we pass generators into the stepper and eliminate the call // signature with primaries @@ -76,9 +80,15 @@ Stepper::Stepper(Input input) CELER_VALIDATE(primaries_action_, << "primary generator was not added to the stepping loop"); + size_type const track_slots = (input.num_track_slots == 0 + ? params_->tracks_per_stream() + : input.num_track_slots); + CELER_VALIDATE(track_slots > 0, + << "track slots were specified neither in core params nor " + "stepper input"); // Create state, including aux data state_ = std::make_shared>( - *params_, input.stream_id, input.num_track_slots); + *params_, input.stream_id, track_slots); // Execute beginning-of-run action ScopedProfiling profile_this{"begin-run"}; diff --git a/src/celeritas/global/Stepper.hh b/src/celeritas/global/Stepper.hh index ee195cf2f1..25bf0298cc 100644 --- a/src/celeritas/global/Stepper.hh +++ b/src/celeritas/global/Stepper.hh @@ -36,6 +36,7 @@ class ActionSequence; * * - \c params : Problem definition * - \c num_track_slots : Maximum number of threads to run in parallel on GPU + * (optional, could be set by params) * \c stream_id : Unique (thread/task) ID for this process * - \c action_times : Whether to synchronize device between actions for timing */ @@ -47,10 +48,7 @@ struct StepperInput bool action_times{false}; //! True if defined - explicit operator bool() const - { - return params && stream_id && num_track_slots > 0; - } + explicit operator bool() const { return params && stream_id; } }; //---------------------------------------------------------------------------// From cdb49c17c5e8f9c0ac0a99cd13e2a380b42c2ae7 Mon Sep 17 00:00:00 2001 From: Seth R Johnson Date: Thu, 23 Jan 2025 13:41:25 -0800 Subject: [PATCH 2/7] Write out core sizes to JSON --- src/celeritas/global/CoreParams.cc | 25 ++++++++++ src/celeritas/global/detail/CoreSizes.json.hh | 48 +++++++++++++++++++ 2 files changed, 73 insertions(+) create mode 100644 src/celeritas/global/detail/CoreSizes.json.hh diff --git a/src/celeritas/global/CoreParams.cc b/src/celeritas/global/CoreParams.cc index 1533a526e2..74882cfa25 100644 --- a/src/celeritas/global/CoreParams.cc +++ b/src/celeritas/global/CoreParams.cc @@ -55,6 +55,8 @@ #include "ActionInterface.hh" +#include "detail/CoreSizes.json.hh" + #if CELERITAS_CORE_GEO == CELERITAS_CORE_GEO_ORANGE # include "orange/OrangeParams.hh" # include "orange/OrangeParamsOutput.hh" @@ -205,6 +207,24 @@ CoreScalars build_actions(ActionRegistry* reg) return scalars; } +//---------------------------------------------------------------------------// +auto get_core_sizes(CoreParams const& cp) +{ + auto const& init = *cp.init(); + + detail::CoreSizes result; + result.streams = cp.max_streams(); + result.processes = comm_world().size(); + + result.initializers = result.streams * init.capacity(); + result.tracks = result.streams * cp.tracks_per_stream(); + result.secondaries = cp.physics()->host_ref().scalars.secondary_stack_factor + * result.tracks; + result.events = init.max_events(); + + return result; +} + //---------------------------------------------------------------------------// } // namespace @@ -311,6 +331,11 @@ CoreParams::CoreParams(Input input) : input_(std::move(input)) input_.output_reg->insert(OutputInterfaceAdapter::from_const_ref( OutputInterface::Category::system, "environ", celeritas::environment())); input_.output_reg->insert(std::make_shared()); + input_.output_reg->insert( + OutputInterfaceAdapter::from_rvalue_ref( + OutputInterface::Category::internal, + "core_sizes", + get_core_sizes(*this))); // Save core diagnostic information input_.output_reg->insert( diff --git a/src/celeritas/global/detail/CoreSizes.json.hh b/src/celeritas/global/detail/CoreSizes.json.hh new file mode 100644 index 0000000000..d92640c73e --- /dev/null +++ b/src/celeritas/global/detail/CoreSizes.json.hh @@ -0,0 +1,48 @@ +//------------------------------- -*- C++ -*- -------------------------------// +// Copyright Celeritas contributors: see top-level COPYRIGHT file for details +// SPDX-License-Identifier: (Apache-2.0 OR MIT) +//---------------------------------------------------------------------------// +//! \file celeritas/global/detail/CoreSizes.json.hh +//---------------------------------------------------------------------------// +#pragma once + +#include "corecel/io/JsonUtils.json.hh" + +namespace celeritas +{ +namespace detail +{ +//---------------------------------------------------------------------------// +/*! + * Save state size/capacity counters. + * + * These should be *integrated* across streams, *per process*. + */ +struct CoreSizes +{ + size_type initializers{}; + size_type tracks{}; + size_type secondaries{}; + size_type events{}; + + size_type streams{}; + size_type processes{}; +}; + +//---------------------------------------------------------------------------// + +void to_json(nlohmann::json& j, CoreSizes const& inp) +{ + j = { + CELER_JSON_PAIR(inp, initializers), + CELER_JSON_PAIR(inp, tracks), + CELER_JSON_PAIR(inp, secondaries), + CELER_JSON_PAIR(inp, events), + CELER_JSON_PAIR(inp, streams), + CELER_JSON_PAIR(inp, processes), + }; +} + +//---------------------------------------------------------------------------// +} // namespace detail +} // namespace celeritas From f2e2445fd2c38610016a0479ca20d8407f8edaf0 Mon Sep 17 00:00:00 2001 From: Seth R Johnson Date: Thu, 23 Jan 2025 13:45:31 -0800 Subject: [PATCH 3/7] Add and fix docs --- src/celeritas/global/CoreParams.cc | 7 ++++++- src/celeritas/global/CoreParams.hh | 4 ++-- src/celeritas/global/CoreState.cc | 5 ++++- src/celeritas/global/CoreState.hh | 2 +- 4 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/celeritas/global/CoreParams.cc b/src/celeritas/global/CoreParams.cc index 74882cfa25..7fa79b27fb 100644 --- a/src/celeritas/global/CoreParams.cc +++ b/src/celeritas/global/CoreParams.cc @@ -213,13 +213,18 @@ auto get_core_sizes(CoreParams const& cp) auto const& init = *cp.init(); detail::CoreSizes result; - result.streams = cp.max_streams(); result.processes = comm_world().size(); + result.streams = cp.max_streams(); + // NOTE: quantities are *per-process* quantities: integrated over streams, + // but not processes result.initializers = result.streams * init.capacity(); result.tracks = result.streams * cp.tracks_per_stream(); + // Number of secondaries is currently based on track size result.secondaries = cp.physics()->host_ref().scalars.secondary_stack_factor * result.tracks; + // Event IDs are the same across all threads so this is *not* multiplied by + // streams result.events = init.max_events(); return result; diff --git a/src/celeritas/global/CoreParams.hh b/src/celeritas/global/CoreParams.hh index a72047143b..b174f9edc9 100644 --- a/src/celeritas/global/CoreParams.hh +++ b/src/celeritas/global/CoreParams.hh @@ -38,8 +38,8 @@ class WentzelOKVIParams; /*! * Global parameters required to run a problem. * - * \todo Always \c tracks_per_stream to build actual states. Currently unit - * tests build states independently. + * \todo Applications specify \c tracks_per_stream to build the states, but + * unit tests currently omit this option. */ class CoreParams final : public ParamsDataInterface { diff --git a/src/celeritas/global/CoreState.cc b/src/celeritas/global/CoreState.cc index 0d7d0fabff..646f872b54 100644 --- a/src/celeritas/global/CoreState.cc +++ b/src/celeritas/global/CoreState.cc @@ -31,7 +31,10 @@ CoreState::CoreState(CoreParams const& params, StreamId stream_id) //---------------------------------------------------------------------------// /*! - * Construct from CoreParams. + * Construct with manual slot count. + * + * This is currently used for unit tests, and temporarily used by the \c + * Stepper constructor. */ template CoreState::CoreState(CoreParams const& params, diff --git a/src/celeritas/global/CoreState.hh b/src/celeritas/global/CoreState.hh index 298b3414b8..c763cbb7c7 100644 --- a/src/celeritas/global/CoreState.hh +++ b/src/celeritas/global/CoreState.hh @@ -86,7 +86,7 @@ class CoreState final : public CoreStateInterface // Construct from CoreParams CoreState(CoreParams const& params, StreamId stream_id); - // Construct witih manual slot count + // Construct with manual slot count CoreState(CoreParams const& params, StreamId stream_id, size_type num_track_slots); From 515ef1f4c837acf393665c2bb352e1b5d5d8fc9a Mon Sep 17 00:00:00 2001 From: Seth R Johnson Date: Thu, 23 Jan 2025 13:54:56 -0800 Subject: [PATCH 4/7] IWYU --- src/celeritas/global/detail/CoreSizes.json.hh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/celeritas/global/detail/CoreSizes.json.hh b/src/celeritas/global/detail/CoreSizes.json.hh index d92640c73e..b6e0291963 100644 --- a/src/celeritas/global/detail/CoreSizes.json.hh +++ b/src/celeritas/global/detail/CoreSizes.json.hh @@ -6,6 +6,9 @@ //---------------------------------------------------------------------------// #pragma once +#include + +#include "corecel/Types.hh" #include "corecel/io/JsonUtils.json.hh" namespace celeritas From b3bd36dfc2d83ae35b6ba86d00ef16c93ecf290e Mon Sep 17 00:00:00 2001 From: Seth R Johnson Date: Thu, 23 Jan 2025 14:33:04 -0800 Subject: [PATCH 5/7] fixup! Add an optional parameter for setting state size via params --- src/celeritas/global/Stepper.cc | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/celeritas/global/Stepper.cc b/src/celeritas/global/Stepper.cc index 067e897a1d..1d93f9ee64 100644 --- a/src/celeritas/global/Stepper.cc +++ b/src/celeritas/global/Stepper.cc @@ -69,10 +69,6 @@ Stepper::Stepper(Input input) return std::make_shared(*params_->action_reg(), opts); }()} { - // NOTE: if this fails, you're probably running from a unit test? - CELER_VALIDATE(params_->tracks_per_stream() > 0, - << "core params `tracks_per_stream` was not set"); - // Save primary action: TODO this is a hack and should be refactored so // that we pass generators into the stepper and eliminate the call // signature with primaries From c17d71d34f7e4533ec892f6d4d3b9440e423da96 Mon Sep 17 00:00:00 2001 From: Seth R Johnson Date: Thu, 23 Jan 2025 14:34:54 -0800 Subject: [PATCH 6/7] Make explicit cast --- src/celeritas/global/CoreParams.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/celeritas/global/CoreParams.cc b/src/celeritas/global/CoreParams.cc index 7fa79b27fb..9f54287060 100644 --- a/src/celeritas/global/CoreParams.cc +++ b/src/celeritas/global/CoreParams.cc @@ -221,8 +221,9 @@ auto get_core_sizes(CoreParams const& cp) result.initializers = result.streams * init.capacity(); result.tracks = result.streams * cp.tracks_per_stream(); // Number of secondaries is currently based on track size - result.secondaries = cp.physics()->host_ref().scalars.secondary_stack_factor - * result.tracks; + result.secondaries = static_cast( + cp.physics()->host_ref().scalars.secondary_stack_factor + * result.tracks); // Event IDs are the same across all threads so this is *not* multiplied by // streams result.events = init.max_events(); From 7876d60db638dad7fb963180c361ae56f657651a Mon Sep 17 00:00:00 2001 From: Seth R Johnson Date: Fri, 24 Jan 2025 07:04:56 -0800 Subject: [PATCH 7/7] Fix clang-tidy warnings --- src/celeritas/optical/Model.hh | 9 +++++++++ src/celeritas/optical/ModelImporter.hh | 3 +++ src/celeritas/optical/Types.hh | 5 +---- src/celeritas/optical/model/AbsorptionModel.cc | 2 +- src/celeritas/optical/model/RayleighModel.cc | 3 ++- 5 files changed, 16 insertions(+), 6 deletions(-) diff --git a/src/celeritas/optical/Model.hh b/src/celeritas/optical/Model.hh index 550118d0e8..cc0266d737 100644 --- a/src/celeritas/optical/Model.hh +++ b/src/celeritas/optical/Model.hh @@ -27,6 +27,15 @@ class MfpBuilder; */ class Model : public OpticalStepActionInterface, public ConcreteAction { + public: + //!@{ + //! \name Type aliases + + //! Function to build optical models with a given action id + using ModelBuilder = std::function(ActionId)>; + + //!@} + public: using ConcreteAction::ConcreteAction; diff --git a/src/celeritas/optical/ModelImporter.hh b/src/celeritas/optical/ModelImporter.hh index cd5070ca43..d3fae21b99 100644 --- a/src/celeritas/optical/ModelImporter.hh +++ b/src/celeritas/optical/ModelImporter.hh @@ -14,6 +14,7 @@ #include "celeritas/io/ImportOpticalModel.hh" +#include "Model.hh" #include "Types.hh" namespace celeritas @@ -56,6 +57,7 @@ class ModelImporter //!@{ //! \name User builder type aliases + using ModelBuilder = Model::ModelBuilder; using UserBuildFunction = std::function(UserBuildInput const&)>; using UserBuildMap = std::unordered_map; @@ -107,6 +109,7 @@ struct WarnAndIgnoreModel //!@{ //! \name Type aliases using UserBuildInput = ModelImporter::UserBuildInput; + using ModelBuilder = ModelImporter::ModelBuilder; //!@} // Warn about a missing optical model and ignore it diff --git a/src/celeritas/optical/Types.hh b/src/celeritas/optical/Types.hh index 22e5c1d95e..ce3bfe4db7 100644 --- a/src/celeritas/optical/Types.hh +++ b/src/celeritas/optical/Types.hh @@ -29,13 +29,10 @@ using ParticleScintSpectrumId = OpaqueId; */ namespace optical { - +//---------------------------------------------------------------------------// //! Alias for MaterialId in core Celeritas namespace using CoreMaterialId = ::celeritas::MaterialId; -//! Function to build optical models with a given action id -using ModelBuilder = std::function(ActionId)>; - //---------------------------------------------------------------------------// } // namespace optical } // namespace celeritas diff --git a/src/celeritas/optical/model/AbsorptionModel.cc b/src/celeritas/optical/model/AbsorptionModel.cc index 7da906667b..0ba9379395 100644 --- a/src/celeritas/optical/model/AbsorptionModel.cc +++ b/src/celeritas/optical/model/AbsorptionModel.cc @@ -19,7 +19,7 @@ namespace optical /*! * Create a model builder for the optical absorption model. */ -ModelBuilder AbsorptionModel::make_builder(SPConstImported imported) +auto AbsorptionModel::make_builder(SPConstImported imported) -> ModelBuilder { CELER_EXPECT(imported); return [i = std::move(imported)](ActionId id) { diff --git a/src/celeritas/optical/model/RayleighModel.cc b/src/celeritas/optical/model/RayleighModel.cc index 46a5740067..3ca450145d 100644 --- a/src/celeritas/optical/model/RayleighModel.cc +++ b/src/celeritas/optical/model/RayleighModel.cc @@ -25,7 +25,8 @@ namespace optical * Create a model builder for Rayleigh scattering from imported data and * material parameters. */ -ModelBuilder RayleighModel::make_builder(SPConstImported imported, Input input) +auto RayleighModel::make_builder(SPConstImported imported, + Input input) -> ModelBuilder { CELER_EXPECT(imported); return [imported = std::move(imported),