eic / epic

DD4hep Geometry Description of the ePIC Experiment
https://eic.github.io/epic
GNU Lesser General Public License v3.0
24 stars 42 forks source link

DD4HEP segmentation bug - GridPhiEta not working #95

Open lkosarz opened 2 years ago

lkosarz commented 2 years ago

Environment: (where does this bug occur, have you tried other environments)

Steps to reproduce: (give a step by step account of how to trigger the bug)

  1. Originally with ATHENA bHCAL
  2. Set segmentation to GridPhiEta
  3. Run simulation

Expected Result: (what do you expect when you execute the steps above)

hit distributions in phi,eta

Actual Result: (what do you get when you execute the steps above)

all hits at eta=0, segmentation fails

Probably still an issue for EPIC

wdconinc commented 1 year ago

After today's discussion in the tutorial, can you edit the description and make this a bit more reproducible? I'd be looking specifically for a branch or something that demonstrates this issue with a minimal set of changes. If this is indeed a bug in the segmentation, it might even make sense to think about a reproducible example that doesn't involve all of EPIC, and instead just uses a dd4hep example as a starting point.

lkosarz commented 1 year ago

Hi Wouter

I missed the tutorial, but I will watch it later. Yes, I will do it with a simple reproducible example. I'm a bit busy, so I need a week or so.

pt., 9 wrz 2022 o 04:29 Wouter Deconinck @.***> napisał(a):

After today's discussion in the tutorial, can you edit the description and make this a bit more reproducible? I'd be looking specifically for a branch or something that demonstrates this issue with a minimal set of changes. If this is indeed a bug in the segmentation, it might even make sense to think about a reproducible example that doesn't involve all of EPIC, and instead just uses a dd4hep example as a starting point.

— Reply to this email directly, view it on GitHub https://github.com/eic/epic/issues/95#issuecomment-1241428047, or unsubscribe https://github.com/notifications/unsubscribe-auth/AKDQ556LJKNBZZJSWER4I73V5KOKDANCNFSM6AAAAAAQCV5WCA . You are receiving this because you authored the thread.Message ID: @.***>

lkosarz commented 1 year ago

Hi Wouter

I did check it with part1 of the old EIC tutorial and found no issue with the GridPhiEta, besides that it assumes R=1. It works, so I didn't find any bug. The only problem is it wasn't very suitable to our need, because R=1. I understand GridRPhiEta will be more suitable for barrel HCal, while PolarGridRPhi2 can be used for endcaps instead.

I made a pull request with the updates: https://github.com/eic/epic/pull/132

I also noticed the old tutorial example needed to be updated to make it work. Please have a look:

https://eicweb.phy.anl.gov/EIC/tutorials/ip6_tutorial_1/-/tree/update_ATHENAtoEPIC

wdconinc commented 1 year ago

I don't understand why R=1 is a restriction. That's the point of using eta and phi, right? That it doesn't matter where you put your detector.

lkosarz commented 1 year ago

The simulation was somehow placing hits at R=1 cm, which actually screwed up position in eta. Eg. I placed the detector disk around 1<eta<2, but the hits ended up at eta=4. I realized that and though that this is just not suitable for a disk geometry - more useful for a barrel where we can set a fixed R.

I see here that any info on the real R coordinate is lost: https://dd4hep.web.cern.ch/dd4hep/reference/classdd4hep_1_1DDSegmentation_1_1GridPhiEta.html#a41efbff4618e7f1a491776529d4f69c0