Skip to content

Conversation

akioogawa
Copy link
Contributor

@akioogawa akioogawa commented May 6, 2025

Briefly, what does this PR introduce?

Remove EcalEndcapPInsert since it will not be built.
See : eic/epic#855 for removing it from geometry.

I'm not sure if I should remove it from :

"EcalEndcapPInsertRawHits",

-PJANADOT:GROUP:EcalEndcapPInsert=edm4hep::SimCalorimeterHit:EcalEndcapPInsertHits,edm4hep::RawCalorimeterHit:EcalEndcapPInsertRawHits,edm4eic::CalorimeterHit:EcalEndcapPInsertRecHits,edm4eic::ProtoCluster:EcalEndcapPInsertIslandProtoClusters,edm4eic::Cluster:EcalEndcapPInsertClusters,edm4eic::MCRecoClusterParticleAssociation:EcalEndcapPInsertClusterAssociations,edm4eic::ProtoCluster:EcalEndcapPInsertTruthProtoClusters,edm4eic::Cluster:EcalEndcapPInsertTruthClusters,edm4eic::MCRecoClusterParticleAssociation:EcalEndcapPInsertTruthClusterAssociations,color_blue

How can I remove it from auto checks?

What kind of change does this PR introduce?

  • Bug fix (issue #__)
  • New feature (issue #__)
  • Documentation update
  • Other: __

Please check if this PR fulfills the following:

  • Tests for the changes have been added
  • Documentation has been added / updated
  • Changes have been communicated to collaborators

Does this PR introduce breaking changes? What changes might users need to make to their code?

No more EcalEndcapPInsert

Does this PR change default behavior?

No more EcalEndcapPInsert

@veprbl
Copy link
Member

veprbl commented Jun 3, 2025

I'm not sure if I should remove it from :

"EcalEndcapPInsertRawHits",

-PJANADOT:GROUP:EcalEndcapPInsert=edm4hep::SimCalorimeterHit:EcalEndcapPInsertHits,edm4hep::RawCalorimeterHit:EcalEndcapPInsertRawHits,edm4eic::CalorimeterHit:EcalEndcapPInsertRecHits,edm4eic::ProtoCluster:EcalEndcapPInsertIslandProtoClusters,edm4eic::Cluster:EcalEndcapPInsertClusters,edm4eic::MCRecoClusterParticleAssociation:EcalEndcapPInsertClusterAssociations,edm4eic::ProtoCluster:EcalEndcapPInsertTruthProtoClusters,edm4eic::Cluster:EcalEndcapPInsertTruthClusters,edm4eic::MCRecoClusterParticleAssociation:EcalEndcapPInsertTruthClusterAssociations,color_blue

Yes, please remove those and other instances too.

How can I remove it from auto checks?

There are codes in
https://github.com/eic/detector_benchmarks/
that rely on EcalEndcapPInsertRecHits and EcalEndcapPInsertClusters, so we need to fix those codes before this can be merged. cc @sebouh137

@veprbl veprbl mentioned this pull request Jun 3, 2025
7 tasks
@sebouh137
Copy link
Contributor

I'm not sure if I should remove it from :

"EcalEndcapPInsertRawHits",

-PJANADOT:GROUP:EcalEndcapPInsert=edm4hep::SimCalorimeterHit:EcalEndcapPInsertHits,edm4hep::RawCalorimeterHit:EcalEndcapPInsertRawHits,edm4eic::CalorimeterHit:EcalEndcapPInsertRecHits,edm4eic::ProtoCluster:EcalEndcapPInsertIslandProtoClusters,edm4eic::Cluster:EcalEndcapPInsertClusters,edm4eic::MCRecoClusterParticleAssociation:EcalEndcapPInsertClusterAssociations,edm4eic::ProtoCluster:EcalEndcapPInsertTruthProtoClusters,edm4eic::Cluster:EcalEndcapPInsertTruthClusters,edm4eic::MCRecoClusterParticleAssociation:EcalEndcapPInsertTruthClusterAssociations,color_blue

Yes, please remove those and other instances too.

How can I remove it from auto checks?

There are codes in https://github.com/eic/detector_benchmarks/ that rely on EcalEndcapPInsertRecHits and EcalEndcapPInsertClusters, so we need to fix those codes before this can be merged. cc @sebouh137

I have created a pull request for this just now; see eic/detector_benchmarks#151

@veprbl veprbl changed the title Removing insert Remove EcalEndcapPInsert Jun 16, 2025
@github-actions github-actions bot added topic: documentation Improvements or additions to documentation topic: infrastructure labels Jun 18, 2025
Copy link
Member

@veprbl veprbl left a comment

Choose a reason for hiding this comment

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

Everything looks good in the diff

@veprbl veprbl added this pull request to the merge queue Jun 18, 2025
@veprbl veprbl removed this pull request from the merge queue due to a manual request Jun 18, 2025
@veprbl
Copy link
Member

veprbl commented Jun 18, 2025

(Let's wait to see if there more comments and merge Tomorrow, otherwise.)

@veprbl veprbl added this pull request to the merge queue Jun 20, 2025
Merged via the queue into main with commit 26f4095 Jun 20, 2025
114 of 118 checks passed
@veprbl veprbl deleted the removeEcalEndcapPInsert branch June 20, 2025 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

topic: calorimetry relates to calorimetry topic: documentation Improvements or additions to documentation topic: forward topic: infrastructure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants