Skip to content

Conversation

@lebuller
Copy link
Contributor

Description

This MR adds a query for checking if the new volume entered in the postBoundaryExecutor is a sensitive detector and outputs the track energy, id, and new volume to screen. To enable this, the SD params are constructed in the core params and passed through to the optical core params. The SDparams data was added to the core track data and a sensitive detector view was added to the core track view to query the SDParams in the postBoundaryExecutor. In addition, the SDParams class was updated to accept an input::detectors at construction instead of a volume id vector

@sethrj sethrj added enhancement New feature or request scoring Hits and track diagnostics labels Oct 27, 2025
Bullerwell, Lance added 2 commits October 31, 2025 14:03
…try and inp::detectors (empty if no detectors) and fix geant setup to load SDs properly
@github-actions
Copy link

github-actions bot commented Oct 31, 2025

Test summary

  499 files    810 suites   27s ⏱️
1 316 tests 1 298 ✅ 14 💤 4 ❌
2 640 runs  2 628 ✅  8 💤 4 ❌

For more details on these failures, see this check.

Results for commit 4953dad.

♻️ This comment has been updated with latest results.

@sethrj
Copy link
Member

sethrj commented Nov 15, 2025

@lebuller If this is ready to review, could you please merge upstream/develop (there's a few conflicts), mark as "ready", and then @stognini can start reviewing? Thanks!

@sethrj sethrj requested a review from stognini November 15, 2025 00:17
@sethrj sethrj changed the title Adds optical SD check and output in post boundary step Add optical SD check and output in post boundary step Nov 19, 2025
{
auto energy = track.particle().energy();
// TODO print energy when killing at SD
std::cout << "Detector hit in volume " << iv_id.get()
Copy link
Member

Choose a reason for hiding this comment

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

Can't have a cout in a CELER_FUNCTION.

{
// Re-entrant into the pre-volume
auto geo = track.geometry();
// auto geo = track.geometry();
Copy link
Member

Choose a reason for hiding this comment

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

Trailing commented line


namespace celeritas
{
//---------------------------------------------------------------------------//
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
//---------------------------------------------------------------------------//


//! Number of detectors
DetectorId::size_type size() const { return volume_ids_.size(); }
DetectorId::size_type size() const { return detectors_.detectors.size(); }
Copy link
Member

Choose a reason for hiding this comment

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

detectors.detectors sounds silly, even though I know what's going on. Just wondering if there is a better naming for the vector in inp::Detectors...

Comment on lines +47 to +48
for (size_type det_num = 0; det_num < detectors_.detectors.size();
++det_num)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
for (size_type det_num = 0; det_num < detectors_.detectors.size();
++det_num)
for (auto det_num : range(detectors_.detectors.size()))

surface_ = std::make_shared<SurfaceParams>(mi.surfaces, *volume_);

detector_ = std::make_shared<SDParams>(*core_geo, mi.detectors);
std::cout << "Detector size " << mi.detectors.detectors.size() << std::endl;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
std::cout << "Detector size " << mi.detectors.detectors.size() << std::endl;

{
namespace optical
{

Copy link
Member

Choose a reason for hiding this comment

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

Missing documentation throughout the file.

@sethrj
Copy link
Member

sethrj commented Nov 20, 2025

@lebuller If this is ready for review (it sounded like it was based on your coments?) please merge in the latest upstream, fix the failing CI tests, and mark as ready for review.

@stognini will take a first crack at it (maybe leave all your comments in a single review rather than indivually?) and then I will take a look too

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request scoring Hits and track diagnostics

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants