Implementation of crosstalk for ALLEGRO ECAL Barrel #82

zwu0922 · 2024-05-14T13:04:01Z

The following PR of k4FWCore has to be processed before this update:
key4hep/k4FWCore#193

New features:

A class for reading the crosstalk map. https://github.com/zwu0922/k4RecCalorimeter/blob/main/RecCalorimeter/src/components/ReadCaloCrosstalkMap.h
The crosstalk is added to the existing "CreateCaloCells" class. The computation of crosstalk can be switched off to speed up the simulation. https://github.com/zwu0922/k4RecCalorimeter/blob/main/RecCalorimeter/src/components/CreateCaloCells.h
Steering file. https://github.com/zwu0922/k4RecCalorimeter/blob/main/RecCalorimeter/tests/options/runEcalBarrel_ReconstructionTopoClusters_crosstalk.py
The output file "output_test_EcalBarrel_crosstalk.root" contains the topo-clusters with crosstalk effect for 5 events.

kjvbrt · 2024-06-24T19:19:50Z

RecCalorimeter/src/components/CreateCaloCells.cpp

@@ -84,13 +90,41 @@ StatusCode CreateCaloCells::execute() {
    m_cellsMap.clear();
  }

+  m_HitCellsMap.clear();


Can you make m_HitCellsMap just a local variable? This way you will not have to clear it in every event.

I changed the name of the variable to "HitCellsMap" and made it local

kjvbrt · 2024-06-24T19:21:06Z

Hi @zwu0922, can you update your branch to the latest upstream?

zwu0922 · 2024-06-25T08:53:45Z

Hi @zwu0922, can you update your branch to the latest upstream?

Done. No conflict needs to be resolved

zwu0922 · 2024-06-25T13:07:33Z

Hi @zwu0922, can you update your branch to the latest upstream?

Done. No conflict needs to be resolved

Looks like the test does not include some recent development in k4FWCore?
key4hep/k4FWCore@5f9a69f

kjvbrt · 2024-06-25T13:13:24Z

Hi @zwu0922 that is not the case, I'm able to compile your PR locally.

The issue seems to be coming from the VolumeManager

CreateECalBarre...WARNING Size of sampling fraction values is smaller than the number of existing layers. Taking the sa
mpling fraction for last layer. Layer ID: 11                                                                           VolumeManager    ERROR lookupContext: Failed to search Volume context 0000004D2FF05804 [Unknown identifier]            
CreateECalBarre...  FATAL  Standard std::exception is caught                                                           
CreateECalBarre...  ERROR VolumeManager: lookupContext: Failed to search Volume context 0000004D2FF05804 [Unknown ident
ifier]                                                                                                                 
EventLoopMgr        FATAL .executeEvent(): Standard std::exception thrown by CreateECalBarrelCells                     
EventLoopMgr        ERROR VolumeManager: lookupContext: Failed to search Volume context 0000004D2FF05804 [Unknown ident
ifier]

giovannimarchiori · 2024-06-25T13:15:03Z

are you using ALLEGRO v03 (11 layers in ECAL barrel) or v01,v02 (12 layers)?

zwu0922 · 2024-06-25T13:22:23Z

are you using ALLEGRO v03 (11 layers in ECAL barrel) or v01,v02 (12 layers)?

I used ALLEGRO v02 (12 layers) to test my code during the development.

kjvbrt · 2024-06-25T13:23:15Z

The test which fails is:

RecFCCeeCalorimeter/tests/options/run_thetamodulemerged.py

With

FCCee/ALLEGRO/compact/ALLEGRO_o1_v03/ALLEGRO_o1_v03.xml

kjvbrt · 2024-06-25T13:24:33Z

RecCalorimeter/tests/options/runEcalBarrel_ReconstructionTopoClusters_crosstalk.py

Can you register this test in RecFCCeeCalorimeter/CMakeLists.txt?

I added the test at the end of CMakeLists.txt

giovannimarchiori · 2024-06-25T13:35:27Z

are you using ALLEGRO v03 (11 layers in ECAL barrel) or v01,v02 (12 layers)?

I used ALLEGRO v02 (12 layers) to test my code during the development.
We have now moved to ALLEGRO v03 (11 layers, aligned cell corners) as baseline

zwu0922 · 2024-06-25T13:42:50Z

are you using ALLEGRO v03 (11 layers in ECAL barrel) or v01,v02 (12 layers)?

I used ALLEGRO v02 (12 layers) to test my code during the development.
We have now moved to ALLEGRO v03 (11 layers, aligned cell corners) as baseline

Thanks, I see. The crosstalk computation does not depend on the number of layers. But of course, I will update the input to the test to match the ALLEGRO v03 design afterwards.

kjvbrt · 2024-06-25T14:14:51Z

Hi @zwu0922, can you adjust the input to the FCCeeLAr_simRecoAllegroV3 test in this PR?

Also, your test FCCeeLAr_runxtalk depends on a variable defined only in LAr_Scripts, can you remove this dependency?

Test #8: FCCeeLAr_runxtalk ................***Failed    1.42 sec
/cvmfs/sw-nightlies.hsf.org/key4hep/releases/2024-06-20/x86_64-almalinux9-gcc11.4.1-opt/k4geo/77e501e47007088f1770eb7009cf1b3684b009f1_develop-4yk2ue/share/k4geo
Traceback (most recent call last):
  File "/home/jsmiesko/zwu0922/k4FWCore/install/bin/k4run", line 241, in <module>
    main()
  File "/home/jsmiesko/zwu0922/k4FWCore/install/bin/k4run", line 185, in main
    load_file(file)
  File "/home/jsmiesko/zwu0922/k4FWCore/install/python/k4FWCore/utils.py", line 51, in load_file
    exec(code, globals())
  File "RecCalorimeter/tests/options/runEcalBarrel_ReconstructionTopoClusters_crosstalk.py", line 260, in <module>
    fileName=os.environ['FCCBASEDIR'] + neighboursMap,
  File "/cvmfs/sw-nightlies.hsf.org/key4hep/releases/2024-05-09/x86_64-almalinux9-gcc11.4.1-opt/python/3.10.13-vrpxgy/lib/python3.10/os.py", line 680, in __getitem__
    raise KeyError(key) from None
KeyError: 'FCCBASEDIR'

BrieucF · 2024-06-25T20:07:33Z

Hi @zwu0922, can you adjust the input to the FCCeeLAr_simRecoAllegroV3 test in this PR?

Also, your test FCCeeLAr_runxtalk depends on a variable defined only in LAr_Scripts, can you remove this dependency?

Test #8: FCCeeLAr_runxtalk ................***Failed    1.42 sec
/cvmfs/sw-nightlies.hsf.org/key4hep/releases/2024-06-20/x86_64-almalinux9-gcc11.4.1-opt/k4geo/77e501e47007088f1770eb7009cf1b3684b009f1_develop-4yk2ue/share/k4geo
Traceback (most recent call last):
  File "/home/jsmiesko/zwu0922/k4FWCore/install/bin/k4run", line 241, in <module>
    main()
  File "/home/jsmiesko/zwu0922/k4FWCore/install/bin/k4run", line 185, in main
    load_file(file)
  File "/home/jsmiesko/zwu0922/k4FWCore/install/python/k4FWCore/utils.py", line 51, in load_file
    exec(code, globals())
  File "RecCalorimeter/tests/options/runEcalBarrel_ReconstructionTopoClusters_crosstalk.py", line 260, in <module>
    fileName=os.environ['FCCBASEDIR'] + neighboursMap,
  File "/cvmfs/sw-nightlies.hsf.org/key4hep/releases/2024-05-09/x86_64-almalinux9-gcc11.4.1-opt/python/3.10.13-vrpxgy/lib/python3.10/os.py", line 680, in __getitem__
    raise KeyError(key) from None
KeyError: 'FCCBASEDIR'

For ALLEGRO v03, the files needed for reconstruction can be found here /eos/project/f/fccsw-web/www/filesForSimDigiReco/ALLEGRO/ALLEGRO_o1_v03/ (see https://github.com/HEP-FCC/FCC-config/tree/main/FCCee/FullSim/ALLEGRO/ALLEGRO_o1_v03#running-the-digitization-and-reconstruction).

Let me know where are the files you need, I will upload them there.

And as you pointed out in a previous discussion, for the tests, you should be able to directly access these files with something like fileName = "https://fccsw.web.cern.ch/fccsw/filesForSimDigiReco/ALLEGRO/ALLEGRO_o1_v03/neighbours_map_ecalB_thetamodulemerged.root"

BrieucF · 2024-07-04T13:13:50Z

Hi @zwu0922 shall we finalize this?

zwu0922 · 2024-07-04T13:36:03Z

Hi @zwu0922 shall we finalize this?

Sorry for the huge delay. I had a problem running the new ddsim for ALLEGRO v03 (Switching off the HCAL simulation always leads to a crash). That is why I didn't manage to adapt my development to ALLEGRO v03. I will contact Tong and others to sort it out as soon as possible and finalise the implementation of cross-talk emulation.

BrieucF · 2024-07-04T13:41:56Z

Ok thanks, let me know when I can copy your files

zwu0922 · 2024-07-08T13:03:32Z

are you using ALLEGRO v03 (11 layers in ECAL barrel) or v01,v02 (12 layers)?

I used ALLEGRO v02 (12 layers) to test my code during the development.
We have now moved to ALLEGRO v03 (11 layers, aligned cell corners) as baseline

Thanks for the notice. I have adapted the test macro to ALLEGRO v03

zwu0922 · 2024-07-08T13:05:37Z

Hi @zwu0922, can you adjust the input to the FCCeeLAr_simRecoAllegroV3 test in this PR?

Also, your test FCCeeLAr_runxtalk depends on a variable defined only in LAr_Scripts, can you remove this dependency?

Test #8: FCCeeLAr_runxtalk ................***Failed    1.42 sec
/cvmfs/sw-nightlies.hsf.org/key4hep/releases/2024-06-20/x86_64-almalinux9-gcc11.4.1-opt/k4geo/77e501e47007088f1770eb7009cf1b3684b009f1_develop-4yk2ue/share/k4geo
Traceback (most recent call last):
  File "/home/jsmiesko/zwu0922/k4FWCore/install/bin/k4run", line 241, in <module>
    main()
  File "/home/jsmiesko/zwu0922/k4FWCore/install/bin/k4run", line 185, in main
    load_file(file)
  File "/home/jsmiesko/zwu0922/k4FWCore/install/python/k4FWCore/utils.py", line 51, in load_file
    exec(code, globals())
  File "RecCalorimeter/tests/options/runEcalBarrel_ReconstructionTopoClusters_crosstalk.py", line 260, in <module>
    fileName=os.environ['FCCBASEDIR'] + neighboursMap,
  File "/cvmfs/sw-nightlies.hsf.org/key4hep/releases/2024-05-09/x86_64-almalinux9-gcc11.4.1-opt/python/3.10.13-vrpxgy/lib/python3.10/os.py", line 680, in __getitem__
    raise KeyError(key) from None
KeyError: 'FCCBASEDIR'

I replaced the test macro for the cross-talk calculation by a modification of the digitisation script for ALLEGRO v03. There is no dependence on LAr_Scripts.

zwu0922 · 2024-07-08T13:07:43Z

Hi @zwu0922, can you adjust the input to the FCCeeLAr_simRecoAllegroV3 test in this PR?
Also, your test FCCeeLAr_runxtalk depends on a variable defined only in LAr_Scripts, can you remove this dependency?
Test #8: FCCeeLAr_runxtalk ................***Failed    1.42 sec
/cvmfs/sw-nightlies.hsf.org/key4hep/releases/2024-06-20/x86_64-almalinux9-gcc11.4.1-opt/k4geo/77e501e47007088f1770eb7009cf1b3684b009f1_develop-4yk2ue/share/k4geo
Traceback (most recent call last):
  File "/home/jsmiesko/zwu0922/k4FWCore/install/bin/k4run", line 241, in <module>
    main()
  File "/home/jsmiesko/zwu0922/k4FWCore/install/bin/k4run", line 185, in main
    load_file(file)
  File "/home/jsmiesko/zwu0922/k4FWCore/install/python/k4FWCore/utils.py", line 51, in load_file
    exec(code, globals())
  File "RecCalorimeter/tests/options/runEcalBarrel_ReconstructionTopoClusters_crosstalk.py", line 260, in <module>
    fileName=os.environ['FCCBASEDIR'] + neighboursMap,
  File "/cvmfs/sw-nightlies.hsf.org/key4hep/releases/2024-05-09/x86_64-almalinux9-gcc11.4.1-opt/python/3.10.13-vrpxgy/lib/python3.10/os.py", line 680, in __getitem__
    raise KeyError(key) from None
KeyError: 'FCCBASEDIR'
For ALLEGRO v03, the files needed for reconstruction can be found here /eos/project/f/fccsw-web/www/filesForSimDigiReco/ALLEGRO/ALLEGRO_o1_v03/ (see https://github.com/HEP-FCC/FCC-config/tree/main/FCCee/FullSim/ALLEGRO/ALLEGRO_o1_v03#running-the-digitization-and-reconstruction).

Let me know where are the files you need, I will upload them there.

And as you pointed out in a previous discussion, for the tests, you should be able to directly access these files with something like fileName = "https://fccsw.web.cern.ch/fccsw/filesForSimDigiReco/ALLEGRO/ALLEGRO_o1_v03/neighbours_map_ecalB_thetamodulemerged.root"

Thanks. Now I point all input root files in the test macro to the central directory for ALLEGRO v03.

zwu0922 and others added 12 commits March 21, 2024 11:45

Add crosstalk neighbour class

d75de82

Use c++ static_cast

0bee6c2

Attempt to add automatic test

9f73623

Adapt to the new headfile name of the k4geo update

0f2f7c8

revert neighbours to ExtSvc in the test

f6695f3

change function name: xtalk_get_cell_indices

e773b7c

Adapt to new k4geo function names for the crosstalk neighbours

2684b5c

Merge branch 'HEP-FCC:main' into main

b1d64e8

Add reading crosstalk neighbours

087b4a1

Add the crosstalk effect to calo cells

1849ecd

Remove unused header

d28a2ab

Implementation of crosstalk

36c1b01

kjvbrt reviewed Jun 24, 2024

View reviewed changes

zwu0922 and others added 2 commits June 25, 2024 10:50

ÂMake HitCellsMap a local variable

dd451b4

Merge branch 'HEP-FCC:main' into main

2dc0a2e

BrieucF assigned BrieucF and kjvbrt Jun 25, 2024

kjvbrt reviewed Jun 25, 2024

View reviewed changes

Add crosstalk computation test in CMakeLists.txt

a3cea53

Zhibo Wu and others added 3 commits July 8, 2024 11:30

Update the test of cross-talk map for ALLEGRO v03

ff6e3c2

Merge branch 'HEP-FCC:main' into main

b61ae59

Update test macros for crosstalk calculation

4ba7f54

BrieucF merged commit 0341395 into HEP-FCC:main Jul 8, 2024
2 of 5 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implementation of crosstalk for ALLEGRO ECAL Barrel #82

Implementation of crosstalk for ALLEGRO ECAL Barrel #82

zwu0922 commented May 14, 2024 •

edited

Loading

kjvbrt Jun 24, 2024

zwu0922 Jun 25, 2024

kjvbrt commented Jun 24, 2024

zwu0922 commented Jun 25, 2024

zwu0922 commented Jun 25, 2024

kjvbrt commented Jun 25, 2024

giovannimarchiori commented Jun 25, 2024

zwu0922 commented Jun 25, 2024

kjvbrt commented Jun 25, 2024

kjvbrt Jun 25, 2024

zwu0922 Jun 25, 2024

giovannimarchiori commented Jun 25, 2024

zwu0922 commented Jun 25, 2024

kjvbrt commented Jun 25, 2024

BrieucF commented Jun 25, 2024

BrieucF commented Jul 4, 2024

zwu0922 commented Jul 4, 2024

BrieucF commented Jul 4, 2024

zwu0922 commented Jul 8, 2024

zwu0922 commented Jul 8, 2024

zwu0922 commented Jul 8, 2024

Implementation of crosstalk for ALLEGRO ECAL Barrel #82

Implementation of crosstalk for ALLEGRO ECAL Barrel #82

Conversation

zwu0922 commented May 14, 2024 • edited Loading

kjvbrt Jun 24, 2024

Choose a reason for hiding this comment

zwu0922 Jun 25, 2024

Choose a reason for hiding this comment

kjvbrt commented Jun 24, 2024

zwu0922 commented Jun 25, 2024

zwu0922 commented Jun 25, 2024

kjvbrt commented Jun 25, 2024

giovannimarchiori commented Jun 25, 2024

zwu0922 commented Jun 25, 2024

kjvbrt commented Jun 25, 2024

kjvbrt Jun 25, 2024

Choose a reason for hiding this comment

zwu0922 Jun 25, 2024

Choose a reason for hiding this comment

giovannimarchiori commented Jun 25, 2024

zwu0922 commented Jun 25, 2024

kjvbrt commented Jun 25, 2024

BrieucF commented Jun 25, 2024

BrieucF commented Jul 4, 2024

zwu0922 commented Jul 4, 2024

BrieucF commented Jul 4, 2024

zwu0922 commented Jul 8, 2024

zwu0922 commented Jul 8, 2024

zwu0922 commented Jul 8, 2024

zwu0922 commented May 14, 2024 •

edited

Loading