Baseline update - #29
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Updates Pinpoint’s baseline detector geometry and associated I/O to match the latest “pinpoint-fortune-geo” layout, including revised scintillator segmentation/hit bookkeeping and updated default macros.
Changes:
- Reworks detector construction to the new Pinpoint/Fortune baseline with aluminum walls, configurable scintillator panels, and IPT layers.
- Extends hit output: scintillator now records both bar-level and pixel-level hits (with
panelIDand orientation), and geometry output now includes tungsten/scint z-position arrays. - Updates generator controls (event range selection, optional pixel-footprint filtering) and refreshes default macros/visualization.
Reviewed changes
Copilot reviewed 20 out of 21 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| Pinpoint/src/ScintSD.cc | Adds bar-level + pixel-level scint hit accumulation and new indexing fields (panel/orientation). |
| Pinpoint/include/ScintSD.hh | Updates ScintSD interface to support a second hits collection and layer indexing configuration. |
| Pinpoint/include/ScintHit.hh | Extends hit payload with panelID, EM-shower/tau provenance, and orientation. |
| Pinpoint/src/PixelSD.cc | Adjusts pixel geometry accessors and layer ID extraction by volume name. |
| Pinpoint/src/PrimaryGeneratorAction.cc | Propagates optional lastEvent to GFaser generator. |
| Pinpoint/include/PrimaryGeneratorAction.hh | Adds static fLastEvent setter/storage. |
| Pinpoint/src/generators/GFaserGeneratorMessenger.cc | Replaces fixed-Z toggle with hitPixelArea option. |
| Pinpoint/include/generators/GFaserGeneratorMessenger.hh | Updates messenger member for the new command. |
| Pinpoint/src/generators/GFaserGenerator.cc | Adds last-event abort, optional skipping of vertices outside pixel footprint, and new z-vertex sampling scheme. |
| Pinpoint/include/generators/GFaserGenerator.hh | Updates generator API/state for lastEvent + hitPixelArea, and new z-vertex helper signature. |
| Pinpoint/src/DetectorConstructionMessenger.cc | Reworks /det UI commands for the new geometry parameterization. |
| Pinpoint/include/DetectorConstructionMessenger.hh | Adds new messenger command members matching the updated geometry controls. |
| Pinpoint/src/DetectorConstruction.cc | Major geometry rebuild: new baseline layout, aluminum walls, scint panel composition, and z-position bookkeeping. |
| Pinpoint/include/DetectorConstruction.hh | Updates geometry accessors/parameters for new baseline (layers, dimensions, z-position getters). |
| Pinpoint/src/AnalysisManager.cc | Updates geometry tree + hit trees (renamed branches, adds scint pixel-hit tree and new geometry branches). |
| Pinpoint/include/AnalysisManager.hh | Adds new geometry/hit output members to match updated ROOT trees. |
| Pinpoint/Pinpoint.cc | Adds --lastEvent CLI option wiring into PrimaryGeneratorAction. |
| Pinpoint/macros/vis.mac | Updates vis macro to load new geom macro and adjusts viewer defaults. |
| Pinpoint/macros/gps.mac | Updates default GPS macro to use new geom macro and revised run/output defaults. |
| Pinpoint/macros/geom.mac | Introduces a dedicated default geometry macro matching the new baseline configuration. |
Comments suppressed due to low confidence (1)
Pinpoint/include/DetectorConstruction.hh:143
- GetScintBarFlag() currently returns 1 unconditionally, so the geometry tree will report bar-geometry even when scint_bar_flag is false. This makes the saved geometry metadata incorrect.
G4int GetScintBarFlag() const {
if (false) {
return 0;
}
if (true) {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| G4int GetNFortuneBlocks() const { return fNFortuneBlocks; } | ||
| void SetNumFortuneBlocks(G4int n) { fNFortuneBlocks = n; } | ||
| G4int GetNPinpointBlocks() const { return fNPinpointBlocks; } | ||
| G4int GetNLayers() const { return fNPinpointBlocks * 2 + fNFortuneBlocks * (fNumScintLayers + 1) + 1; } |
Contributor
Author
There was a problem hiding this comment.
@tobias-boeckh, should GetNLayers() include the IPT?
| // Scintillator geometry | ||
| scintDetectorWidth = det->GetScintDetectorWidth() / mm; | ||
| scintDetectorHeight = det->GetScintDetectorHeight() / mm; | ||
| scintThickness = det->GetScintLayerThickness() / mm; |
Comment on lines
38
to
47
| for (G4int i = 0; i < argc; i = i + 2) { | ||
| G4String g4argv(argv[i]); // convert only once | ||
| if (g4argv == "-p") physListName = argv[i + 1]; | ||
| else if (g4argv == "-f" || g4argv == "--firstEvent") { | ||
| firstEvent = std::atol(argv[i + 1]); | ||
| } | ||
| else if (g4argv == "-l" || g4argv == "--lastEvent") { | ||
| lastEvent = std::atol(argv[i + 1]); | ||
| } | ||
| } |
Prevent nScintillators per layer being > 0 when scintillators layers == 0 Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Contributor
Author
|
I think this is ready to merge now - these are some event displays for a through going muon and a tau neutrino |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
My fork of
pinpoint-fortune-geoto address issues/28Should replicate the new baseline geometry in the collaboration meeting [https://indico.cern.ch/event/1708260/contributions/7184825/attachments/3311775/5927104/pinpoint_fortune_geometry_render_2.pdf]
Fixes the pixel copy number issue identified by Joern
Fixes aluminium walls overlapping with mother volume
updated macros (geometry now moved to separate macro which is then loaded by steering macros)
Scintillator size updated to 45 x 45 cm
Renamed the branches in the pixel hits tree to be consistent with the other hits trees (i.e. dropped the
hit_prefix on the branch names)Added
panelIDbranch to the scintillator hits trees. This branch encodes an index which can be used with thescint_Zposbranch in the geometry to get the z-positions of each scintillator layer in the detector.Updated event display notebook