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

Construct CoreParams from new problem input in celer-sim #1601

Open
wants to merge 32 commits into
base: develop
Choose a base branch
from

Conversation

sethrj
Copy link
Member

@sethrj sethrj commented Jan 29, 2025

This follow-on to #1562 moves the core construction from app/celer-sim to celeritas/setup. I removed a couple of small capabilities related to the deprecated celer-sim json input for now:

  • We no longer echo the JSONified input to the root output file
  • If something goes wrong during problem setup, we no longer echo the "processed" JSON input

Changed in inp:

  • Split inp::StateCapacity into core/optical
  • Restored full eloss spline order option
  • Made a few "ignored if zero" options std::optional rather than sentinel values

@sethrj sethrj added minor Minor internal changes or fixes app Application front ends labels Jan 29, 2025
@sethrj sethrj requested a review from amandalund January 29, 2025 14:35
Copy link

github-actions bot commented Jan 29, 2025

Test summary

 4 497 files   6 923 suites   13m 11s ⏱️
 1 698 tests  1 692 ✅  6 💤 0 ❌
24 285 runs  24 201 ✅ 84 💤 0 ❌

Results for commit 82ee9c6.

♻️ This comment has been updated with latest results.

@amandalund
Copy link
Contributor

Currently I'm hitting an error trying to run celer-sim on the GPU:

/home/alund/celeritas_project/celeritas/app/celer-sim/celer-sim.cc:273: critical: While running input at simple-cms-gpu.inp.json: celeritas: runtime error: celeritas::activate_device may be called only once per application
/home/alund/celeritas_project/celeritas/src/corecel/sys/Device.cc:308: '!d' failed

@sethrj
Copy link
Member Author

sethrj commented Feb 1, 2025

Glad someone here is paying attention 😅 I'll check it out on wildstyle and fix.

@sethrj
Copy link
Member Author

sethrj commented Feb 6, 2025

@amandalund all tests pass on a debug+openmp+cuda build on wildstyle!

@amandalund
Copy link
Contributor

Awesome, thanks! I'll get back to this today.

Copy link
Contributor

@amandalund amandalund 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 good @sethrj! One kind of odd thing is the results on this branch were different from develop (but still statistically the same) when I tried running the cms2018 problem below. Is there any reason the results might have changed?

cms2018 json input
{
 "geometry_file": "/home/alund/celeritas_project/regression/input/cms2018.gdml",
 "physics_options": {
  "coulomb_scattering": false,
  "eloss_fluctuation": true,
  "em_bins_per_decade": 56,
  "lpm": true,
  "msc": "urban",
  "physics": "em_basic",
  "rayleigh_scattering": true
 },
 "brem_combined": false,
 "primary_options": {
  "seed": 12345,
  "direction": {"distribution": "isotropic"},
  "energy": 10000,
  "num_events": 8,
  "pdg": 11,
  "position": [0, 0, 0],
  "primaries_per_event": 130
 },
 "initializer_capacity": 8388608,
 "num_track_slots": 65536,
 "field": [0.0, 0.0, 1.0],
 "step_diagnostic": true,
 "step_diagnostic_bins": 1000,
 "max_events": 1024,
 "secondary_stack_factor": 3.0,
 "seed": 20220905,
 "merge_events": false,
 "action_times": false,
 "use_device": false
}

// celeritas::device is reset)
if (celeritas::device())
{
celeritas::device().create_streams(0);
Copy link
Contributor

Choose a reason for hiding this comment

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

We also do this in SharedParams, maybe we should have a Device::destroy_streams()?

@@ -207,6 +265,9 @@ inp::StandaloneInput to_input(RunnerInput const& ri)
si.problem = load_problem(ri);
if (!ri.physics_file.empty())
{
CELER_VALIDATE(
ends_with(ri.physics_file, ".root"),
<< R"(physics_file must be a ROOT input: use GDML for geometry_file and if forcing an ORANGE geometry, use the `ORANGE_FORCE_INPUT` environment variable)");
Copy link
Contributor

Choose a reason for hiding this comment

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

Should GDML physics/org.json ORANGE be deprecated input formats? Or are we not worrying about backward compatibility here.

return EventReader{rfe.event_file, nullptr}.num_events();
},
},
si.events);
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we're getting the number of events here, could we also update the number of streams to not be more than the number of events?

namespace setup
{
//---------------------------------------------------------------------------//
struct StandaloneLoaded
Copy link
Contributor

Choose a reason for hiding this comment

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

How will this be used?

if constexpr (CELERITAS_CORE_GEO == CELERITAS_CORE_GEO_ORANGE)
{
static char const fi_hack_envname[] = "ORANGE_FORCE_INPUT";
auto const& filename = celeritas::getenv(fi_hack_envname);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this hack necessary?


//---------------------------------------------------------------------------//
/*!
* Costruct magnetic field from variant input.
Copy link
Contributor

Choose a reason for hiding this comment

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

Update description.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
app Application front ends minor Minor internal changes or fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants