Skip to content

12-Sector-CyMBaL - #2829

Open
ybedfer wants to merge 13 commits into
mainfrom
12-Sector-CyMBaL-compatible
Open

12-Sector-CyMBaL#2829
ybedfer wants to merge 13 commits into
mainfrom
12-Sector-CyMBaL-compatible

Conversation

@ybedfer

@ybedfer ybedfer commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Briefly, what does this PR introduce? Please link to any relevant presentations or discussions.

New version of MPGD plugin and digitization compatible w/ upcoming 12-Sector-CyMBaL (see "https://github.com/eic/epic/tree/12-Sector-CyMBaL").

What is the urgency of this PR?

  • [ *] Medium

What kind of change does this PR introduce?

  • [* ] New feature (no related issue ever created)

Please check if any of the following apply

None.

ybedfer added 2 commits August 1, 2026 11:24
… DD4hep #f7904caf.

- Digitization;
+ Add the 12-sector CyMBaL segmentation scheme to the list of valid schemes.
+ DeadZone rejection: if enabled, applies simultaneoulsy to both 2DStrips.
 Active being #strips * pitch.
- Plugin (if "MPGDTrackerDigi" factory):
+ Resolutions (config parameter) read from XML, if available.
+ New "hasDeadZone" boolean config parameter (set =true for CyMBaL).
@github-actions github-actions Bot added topic: tracking Relates to tracking reconstruction topic: barrel topic: digitization labels Aug 4, 2026
@veprbl

veprbl commented Aug 4, 2026

Copy link
Copy Markdown
Member

As usual, we will need the PR for epic geometry as well. And as before, I would really appreciate any help with reviewing this.

I assume this is backwards compatible? Or we just rely on the fact that strip geometry is not used in the production?

@ybedfer

ybedfer commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

This is the first one of a series of PRs meant to commit a 12-sector version of CyMBaL.

It's brought into play only if the epic_craterlake_tracking_2DStrip configuration is processed. => Default behavior of eicrecon is not affected.

When epic_craterlake_tracking_2DStrip is processed, it's backward compatible. But obviously requires a new version of epic, w/ modified XML and detector builders for CyMBaL (i.e. MPGDBarrelHits) for the new features to be activated.
It then requires a version of DD4hep >= #f7904caf, i.e. :
"DetectorSensitives: assign sensitive detector to reflected object if the logical volume was sensitive".

Then it does two things:

  • Update MPGDTrackerDigi, to get it to understand the segmentation scheme of the new CyMBaL.
  • Update the MPGD plugin, to include new configuration parameters.

Below a plot illustrating backward compatibility: it's SimHit and RecHit-SimHit residual distributions, obtained w/ an 8-Sector CyMBaL:
8SCyMBaL SimHits
8SCyMBaL Residuals

Note that I have not been able to run the hit-matching macro: it crashes, first upon encountering TOF RecHits, then elsewhere if TOF is bypassed.

@veprbl

veprbl commented Aug 4, 2026

Copy link
Copy Markdown
Member

Note that I have been able to run the hit-matching macro: it crashes, first upon encountering TOF RecHits, then elsewhere if TOF is bypassed.

This reminds me of a known regression from three weeks ago that should be fixed on current main via #2745. Have you tested with most up to date version of your branch?

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Clang-Tidy found issue(s) with the introduced code (1/2)

Comment thread src/algorithms/digi/MPGDTrackerDigi.cc Outdated
Comment thread src/algorithms/digi/MPGDTrackerDigi.cc Outdated
Comment thread src/algorithms/digi/MPGDTrackerDigi.cc
Comment thread src/algorithms/digi/MPGDTrackerDigi.cc Outdated
Comment thread src/algorithms/digi/MPGDTrackerDigi.cc Outdated
Comment thread src/algorithms/digi/MPGDTrackerDigi.cc Outdated
Comment thread src/algorithms/digi/MPGDTrackerDigi.cc Outdated
Comment thread src/algorithms/digi/MPGDTrackerDigi.cc Outdated
Comment thread src/algorithms/digi/MPGDTrackerDigi.cc Outdated
Comment thread src/algorithms/digi/MPGDTrackerDigi.cc Outdated

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Clang-Tidy found issue(s) with the introduced code (2/2)

Comment thread src/algorithms/digi/MPGDTrackerDigi.cc Outdated
Comment thread src/algorithms/digi/MPGDTrackerDigi.cc Outdated
Comment thread src/algorithms/digi/MPGDTrackerDigi.cc Outdated
Comment thread src/algorithms/digi/MPGDTrackerDigi.cc Outdated
Comment thread src/algorithms/digi/MPGDTrackerDigi.cc Outdated
Comment thread src/algorithms/digi/MPGDTrackerDigi.cc Outdated
Comment thread src/algorithms/digi/MPGDTrackerDigi.cc Outdated
Comment thread src/algorithms/digi/MPGDTrackerDigi.h Outdated
Comment thread src/algorithms/digi/MPGDTrackerDigi.cc Outdated
Comment thread src/algorithms/digi/MPGDTrackerDigi.cc Outdated
ybedfer and others added 3 commits August 5, 2026 15:40
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Clang-Tidy found issue(s) with the introduced code (1/1)

Comment thread src/algorithms/digi/MPGDTrackerDigi.cc Outdated
Comment thread src/algorithms/digi/MPGDTrackerDigi.cc Outdated
epic-capybara and others added 3 commits August 5, 2026 09:53
This PR applies the include-what-you-use fixes as suggested by
https://github.com/eic/EICrecon/actions/runs/31011440068.
Please merge this PR into the branch `12-Sector-CyMBaL-compatible`
to resolve failures in PR #2829.

Auto-generated by [create-pull-request][1]

[1]: https://github.com/peter-evans/create-pull-request

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@ybedfer

ybedfer commented Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

Hello,

Note that I have been able to run the hit-matching macro: it crashes, first upon encountering TOF RecHits, then elsewhere if TOF is bypassed.

This reminds me of a known regression from three weeks ago that should be fixed on current main via #2745. Have you tested with most up to date version of your branch?

Everything was up-to-date.
I tried again. => Same outcome: No TOF RecHit, and bypassing TOF does not help: something is stored in the "CentralCKFTracks" collection that I cannot identify since macro crashes as soon as I try to access its cellID.

(I managed to find a work around the absence of TOF RecHit, see PR "eic/epic#1146" of epic.)

@ybedfer

ybedfer commented Aug 8, 2026

Copy link
Copy Markdown
Contributor Author

To ease the task of the reviewers, here is a breakdown of the modifications to the source code:

TWO files, mainly, are concerned:

I) "MPGDTrackerDigi": THREE sets of modifications:
i) Method "parseSegmentation":

  • Extend the set of valid segmentation schemes to include 12SCyMBaL.
  • In order to not duplicate source code lines, I created a lambda function
    for setting "StripParameters", viz.: "getGridPhiZParams".

ii) DeadZone:

  • Its handled by new member method "isInActive".
  • Conditioning the generation of clusters.
  • Conditioned by a new config. parameter: "hasDeadZone".

iii) Fulfilling requests from github actions, like "gsl::at", avoid casting
array into pointer, and, so-called, readability improvements.

II) MPGD plugin: configuration parameters:
i) "stripResolutions": set from an XML <constant>.
ii) "hasDeadZone": false by default, set =true for CyMBaL.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants