Fix hits event small event binning#130
Merged
Merged
Conversation
Reverts commit 5c7b983, which changed the default DrawEvent option from "hist(Cont1,col)" to "hist(Cont1,col)[3]". That "[3]" sets a fixed 3 mm pitch, which triggers the `if (pitch > 0)` branch in DrawHistograms and overrides the adaptive bin count that TRestHits::GetBoundaries already computes from the data spacing. With the fixed pitch, nBins = round(range/3) rounds to 0-1 bins for events smaller than 3 mm, collapsing small/localized events (e.g. X-ray hits) into a single solid band. Removing the "[3]" restores pitch = 0, so the override is skipped and the existing adaptive GetBoundaries binning is used again. I have not been able to determine the original purpose of the "[3]" pitch. If anyone knows why it was added, or disagrees with this revert, please let me know. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
TRestHits::GetBoundaries reads dist.front()/back(), which is undefined behaviour when a projection vector is empty (an event with no hits of a given type, e.g. only XZ hits and no Y-defining hits). Wrap it in a small helper that returns a safe default range and one bin for an empty projection. Also clamp nBins to at least 1 after the optional pitch override, so a degenerate event (all hits sharing a coordinate, or a sub-pitch extent) can never request a 0-bin histogram. These guards do not change the rendering of normal events. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add bottom and left margins to each sub-pad so the axis titles are no longer cut off (most visibly the bottom-right Z histogram). When an event has no XY projection (strip readouts with XZ/YZ hits), use the empty top-right pad for a 3D view. Each strip hit is drawn as a line along its undetermined axis (XZ -> Y, YZ -> X, XY -> Z), colored by energy. Where an XZ and a YZ line share the same Z they cross, and that crossing is the reconstructed hit position, marked with a dot. This draws the event in 3D. With many strips at the same Z some crossings are ambiguous "ghost" points, which is normal for strip readouts. The 3D objects are owned by the pad, so they are freed on the next draw. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
for more information, see https://pre-commit.ci
cmargalejo
changed the title
cmargalejo
requested review from
AlvaroEzq,
JPorron,
hgmaluenda,
juanangp and
mariajmz
June 19, 2026 16:41
mariajmz
approved these changes
Jun 19, 2026
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
2 participants
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.
Summary
The event browser drew small, localized
TRestDetectorHitsEvents (e.g. X-raycalibration hits) as solid colored vertical bands instead of a recognizable hit
cluster. This PR fixes that, hardens the drawing code, and improves the
DrawEventlayout.What changed (3 commits)
Revert default DrawEvent pitch to use the existing adaptive binning
⚠️ I could not find the original purpose of the
Reverts commit
5c7b983d, which set the default option tohist(Cont1,col)[3].The
[3]forces a 3 mm bin pitch, sonBins = round(range/3)rounds to 0–1 binsfor events smaller than 3 mm, collapsing them into a single band. Removing it
restores the adaptive binning that
TRestHits::GetBoundariesalready computes.[3]pitch. If anyone knowswhy it was added, or disagrees with this revert, please let me know.
Guard DrawHistograms against degenerate events
GetBoundariesreadsfront()/back(), undefined for an empty projection (anevent with no hits of a given type) — a latent crash. Also clamp
nBins >= 1.No effect on normal events.
Fix pad layout and add a 3D view for strip events
Add sub-pad margins so axis titles are no longer cut off (most visibly the
bottom-right Z histogram). When there is no XY projection (strip readouts with
XZ/YZ hits), use the otherwise-empty top-right pad for a 3D view: each strip hit
is a line along its undetermined axis (XZ→Y, YZ→X, XY→Z), colored by energy, and
the XZ/YZ crossings (reconstructed hit positions) are marked with dots — which
draws the event in 3D.
Notes
DrawEvent/DrawHistograms); the analysis-facingGetXYHistogram/GetXZHistogram/GetYZHistogramare untouched.high-multiplicity events — inherent to strip readouts, not a bug.
Before / after
Before — a small X-ray event (572900) collapses into solid vertical bands, and the
bottom-right Z histogram is clipped:
After — same event: adaptive binning, full axis titles, and the new 3D view with the
strip-crossing markers (reconstructed positions):
After — an alpha event: the 3D crossings trace out the track cloud: