update sliding-window clustering for Theta-Module readout #58

dasphy · 2023-11-08T18:42:37Z

Hello,
I create this PR for making sliding-window clustering work with Theta-Module readout.
Please help me review the code, thank you!

Related PRs:
key4hep/k4FWCore#158
BrieucF/LAr_scripts#16

@BrieucF
@giovannimarchiori
@gartrog

Cheers,
Tong Li

BrieucF · 2023-11-14T08:35:48Z

Hello Tong, this will break the FCC-hh clustering. I would propose to create new algorithms under RecFCCeeCalorimeter. What do you think @kjvbrt ?

BrieucF · 2023-11-14T08:52:16Z

RecCalorimeter/src/components/CaloTowerTool.cpp

@@ -23,6 +23,9 @@ CaloTowerTool::CaloTowerTool(const std::string& type, const std::string& name, c
  declareProperty("hcalExtBarrelCells", m_hcalExtBarrelCells, "");
  declareProperty("hcalEndcapCells", m_hcalEndcapCells, "");
  declareProperty("hcalFwdCells", m_hcalFwdCells, "");
+  declareProperty("positionsECalBarrelTool", m_cellPositionsECalBarrelTool,
+                  "Handle for tool to retrieve cell positions in ECal Barrel");


Is there a way to not introduce the dependency on the Cell Position tools?

Do you need to adjust cell positions or you asume they are not positioned at all?

I need to get the cell positions in theta and phi. For theta, we can also use "segmentation->theta(cellID)" from segmentation. However "segmentation->phi(cellID)" only returns a relative shift in phi, not the actual phi position of a cell. So @giovannimarchiori suggested me to use the Cell Position Tool.
What do you think? @giovannimarchiori

The cell positions are needed to find out to which tower in eta-phi they belong, but the position does not need to be modified. So probably one could just run the clustering on the positioned cells and there would be no need for the positioning tool here.
@dasphy this means that you should use "ECalBarrelPositionedCells" rather than EcalBarrelCellsName (= "ECalBarrelCells") for towers.ecalBarrelCells.Path in https://github.com/BrieucF/LAr_scripts/pull/16/files#diff-58bdd8a8f3d731a4b580bf79c17c026b7ccd59c296d01a3c1aa01b02ffd6a4cf
Can you try to use the positioned cells (and retrieve theta/phi directly from their position) rather than from the positioning tool or the segmentation?

That's a good idea, thanks @giovannimarchiori

RecCalorimeter/src/components/CaloTowerTool.cpp

kjvbrt · 2023-11-14T09:20:09Z

Hello Tong, this will break the FCC-hh clustering. I would propose to create new algorithms under RecFCCeeCalorimeter. What do you think @kjvbrt ?

I agree, since RecFCCeeCalorimeter theta based readout is relevant only for the FCC-ee version.

dasphy · 2023-11-14T13:43:41Z

Hello Tong, this will break the FCC-hh clustering. I would propose to create new algorithms under RecFCCeeCalorimeter. What do you think @kjvbrt ?

I agree, since RecFCCeeCalorimeter theta based readout is relevant only for the FCC-ee version.

OK.

dasphy · 2023-12-10T21:57:26Z

Hello @BrieucF , @kjvbrt ,
I made some changes in the PR:

Created code for FCCee SW clustering with ThetaModule readouts in "RecFCCeeCalorimeter"
Left the code for FCChh as it is
Removed "CellPositioTool" in SW clustering
Created a new IToweTool for FCCee in "k4FWCore/":
update sliding-window clustering for Theta-Module readout key4hep/k4FWCore#158

Let me know if there are any issues, thanks!

@giovannimarchiori
@gartrog

Cheers,
Tong Li

kjvbrt · 2023-12-11T14:45:47Z

Hi @dasphy,

when trying out the PR I got the following error:

ToolSvc.towers       INFO Readout ECalEndcapPhiEta found.
ToolSvc.towers      ERROR There is no module-theta or multi- segmentation for the readout ECalEndcapPhiEta 
defined.
ToolSvc.towers       INFO Retrieving Ecal forward segmentation
ToolSvc.towers       INFO Readout does not exist! Please check if it is correct. Processing without it.
ToolSvc.towers       INFO Retrieving Hcal barrel segmentation
ToolSvc.towers       INFO Readout HCalBarrelReadout found.
ToolSvc.towers      ERROR There is no module-theta or multi- segmentation for the readout HCalBarrelReadout
 defined.
ToolSvc.towers       INFO Retrieving Hcal extended barrel segmentation
ToolSvc.towers       INFO Readout does not exist! Please check if it is correct. Processing without it.
ToolSvc.towers       INFO Retrieving Hcal endcap segmentation
ToolSvc.towers       INFO Readout does not exist! Please check if it is correct. Processing without it.
ToolSvc.towers       INFO Retrieving Hcal forward segmentation
ToolSvc.towers       INFO Readout does not exist! Please check if it is correct. Processing without it.
ToolSvc.towers       INFO == Retrieving segmentation ECalBarrelModuleThetaMerged
CreateClusters       INFO CreateCaloClustersSlidingWindowFCCee initialized

Is it because it also depends on #59 ?

RecFCCeeCalorimeter/src/components/CaloTowerToolFCCee.cpp

dasphy · 2023-12-11T14:57:26Z

Hi @kjvbrt ,
No, it doesn't depend on #59 .

Please run the FCCee SW clustering using run_thetamodulemerged.py
rather than runTopoAndSlidingWindowAndCaloSim.py.

Best,
Tong

Hi @dasphy,

when trying out the PR I got the following error:

ToolSvc.towers       INFO Readout ECalEndcapPhiEta found.
ToolSvc.towers      ERROR There is no module-theta or multi- segmentation for the readout ECalEndcapPhiEta 
defined.
ToolSvc.towers       INFO Retrieving Ecal forward segmentation
ToolSvc.towers       INFO Readout does not exist! Please check if it is correct. Processing without it.
ToolSvc.towers       INFO Retrieving Hcal barrel segmentation
ToolSvc.towers       INFO Readout HCalBarrelReadout found.
ToolSvc.towers      ERROR There is no module-theta or multi- segmentation for the readout HCalBarrelReadout
 defined.
ToolSvc.towers       INFO Retrieving Hcal extended barrel segmentation
ToolSvc.towers       INFO Readout does not exist! Please check if it is correct. Processing without it.
ToolSvc.towers       INFO Retrieving Hcal endcap segmentation
ToolSvc.towers       INFO Readout does not exist! Please check if it is correct. Processing without it.
ToolSvc.towers       INFO Retrieving Hcal forward segmentation
ToolSvc.towers       INFO Readout does not exist! Please check if it is correct. Processing without it.
ToolSvc.towers       INFO == Retrieving segmentation ECalBarrelModuleThetaMerged
CreateClusters       INFO CreateCaloClustersSlidingWindowFCCee initialized

Is it because it also depends on #59 ?

RecFCCeeCalorimeter/src/components/CaloTowerToolFCCee.cpp

dasphy · 2023-12-12T12:28:29Z

Hi @kjvbrt
if you saw some errors when compile like:

[ 11%] Building CXX object RecFCCeeCalorimeter/CMakeFiles/k4RecFCCeeCalorimeterPlugins.dir/src/components/CaloTowerToolFCCee.cpp.o
[93](https://github.com/HEP-FCC/k4RecCalorimeter/actions/runs/7180539586/job/19555067061#step:5:94)
In file included from /Package/RecFCCeeCalorimeter/src/components/CaloTowerToolFCCee.cpp:1:
[94](https://github.com/HEP-FCC/k4RecCalorimeter/actions/runs/7180539586/job/19555067061#step:5:95)
/Package/RecFCCeeCalorimeter/src/components/CaloTowerToolFCCee.h:10:10: fatal error: k4Interface/ITowerToolThetaModule.h: No such file or directory
[95](https://github.com/HEP-FCC/k4RecCalorimeter/actions/runs/7180539586/job/19555067061#step:5:96)
   10 | #include "k4Interface/ITowerToolThetaModule.h"
[96](https://github.com/HEP-FCC/k4RecCalorimeter/actions/runs/7180539586/job/19555067061#step:5:97)
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[97](https://github.com/HEP-FCC/k4RecCalorimeter/actions/runs/7180539586/job/19555067061#step:5:98)
compilation terminated.
[98](https://github.com/HEP-FCC/k4RecCalorimeter/actions/runs/7180539586/job/19555067061#step:5:99)
make[2]: *** [RecFCCeeCalorimeter/CMakeFiles/k4RecFCCeeCalorimeterPlugins.dir/src/components/CaloTowerToolFCCee.cpp.o] Error 1
[99](https://github.com/HEP-FCC/k4RecCalorimeter/actions/runs/7180539586/job/19555067061#step:5:100)
make[2]: *** Waiting for unfinished jobs....

Because it depends on this PR:
key4hep/k4FWCore#158

Cheers, Tong

kjvbrt · 2023-12-12T15:02:54Z

Hi @dasphy,

I'm still not able to run the modified run_thetamodulemerged.py.

I'm getting:

*** Error in `python': malloc(): memory corruption (fast): 0x0000000026d608e0 ***
Segmentation fault

dasphy · 2023-12-13T15:07:10Z

Hi @dasphy,

I'm still not able to run the modified run_thetamodulemerged.py.

I'm getting:
*** Error in `python': malloc(): memory corruption (fast): 0x0000000026d608e0 ***
Segmentation fault

Sorry I haven't seen this before.
I just saw #59 is merged. I need to modify a bit my code to solve the conflicts.

Tong

dasphy · 2023-12-13T15:52:14Z

Hi @kjvbrt , @BrieucF
do you know who are supposed to review this?
key4hep/k4FWCore#158

Tong

dasphy · 2023-12-13T23:17:36Z

Hi @dasphy,
I'm still not able to run the modified run_thetamodulemerged.py.
I'm getting:
*** Error in `python': malloc(): memory corruption (fast): 0x0000000026d608e0 ***
Segmentation fault
Sorry I haven't seen this before. I just saw #59 is merged. I need to modify a bit my code to solve the conflicts.

Tong

SW-clustering code in this PR also updated with k4geo.

dasphy · 2023-12-14T16:35:34Z

Hi @dasphy,
I'm still not able to run the modified run_thetamodulemerged.py.
I'm getting:
*** Error in `python': malloc(): memory corruption (fast): 0x0000000026d608e0 ***
Segmentation fault
Sorry I haven't seen this before. I just saw #59 is merged. I need to modify a bit my code to solve the conflicts.
Tong
SW-clustering code in this PR also updated with k4geo.

Hi @kjvbrt , @BrieucF ,
I just update my code with the latest version (including PR #59 ),
the FCCee SW clustering can run properly, without error you saw.

run_thetamodulemerged.py script is updated:
BrieucF/LAr_scripts#18

this one is closed & discarded:
BrieucF/LAr_scripts#16

Tong

dasphy · 2023-12-22T17:38:52Z

Hi @dasphy,
I'm still not able to run the modified run_thetamodulemerged.py.
I'm getting:
*** Error in `python': malloc(): memory corruption (fast): 0x0000000026d608e0 ***
Segmentation fault
SW-clustering code in this PR also updated with k4geo.
Hi @kjvbrt , @BrieucF , I just update my code with the latest version (including PR #59 ), the FCCee SW clustering can run properly, without error you saw.

run_thetamodulemerged.py script is updated: BrieucF/LAr_scripts#18
this one is closed & discarded: BrieucF/LAr_scripts#16
Tong

Hello @kjvbrt , @BrieucF ,
Just want to ping you for this PR before the winter break.
The code was updated with k4geo and working well from my side.
Cheers & Happy Holidays,
Tong

kjvbrt · 2024-01-24T14:03:24Z

Hi @dasphy, sorry for the late reply, it took me a while to find source of the problem I was encountering in your PR.

Can you move struct cluster from global namespace into the algorithm class? The same way as is done in this PR:
#64

dasphy · 2024-01-24T16:12:39Z

Hi @dasphy, sorry for the late reply, it took me a while to find source of the problem I was encountering in your PR.

Can you move struct cluster from global namespace into the algorithm class? The same way as is done in this PR: #64

Done. Thanks!
Tong

RecFCCeeCalorimeter/src/components/CaloTowerToolFCCee.h

dasphy · 2024-02-01T16:15:19Z

Hello @kjvbrt , @jmcarcell ,
I implemented your comments in this PR, as well as in PR key4hep/k4FWCore#158
I just compiled the updated packages, and "run_thetamodulemerged.py" for clustering works well.
Let me know if there's any issue.

Tagging: @giovannimarchiori @gartrog

Best, Tong

update sliding-window clustering for Theta-Module readout

8dfc612

This was referenced Nov 9, 2023

update sliding-window clustering for Theta-Module readout BrieucF/LAr_scripts#16

Closed

update sliding-window clustering for Theta-Module readout key4hep/k4FWCore#158

Merged

BrieucF reviewed Nov 14, 2023

View reviewed changes

kjvbrt reviewed Nov 14, 2023

View reviewed changes

RecCalorimeter/src/components/CaloTowerTool.cpp Outdated Show resolved Hide resolved

mv SW code to RecFCCeeCalorimeter/

ccfcae8

kjvbrt reviewed Dec 11, 2023

View reviewed changes

RecFCCeeCalorimeter/src/components/CaloTowerToolFCCee.cpp Outdated Show resolved Hide resolved

small fix SW clustering FCCee

dde75fa

kjvbrt reviewed Dec 12, 2023

View reviewed changes

RecFCCeeCalorimeter/src/components/CaloTowerToolFCCee.cpp Show resolved Hide resolved

kjvbrt reviewed Dec 12, 2023

View reviewed changes

RecFCCeeCalorimeter/src/components/CaloTowerToolFCCee.cpp Outdated Show resolved Hide resolved

fix SW clustering FCCee

5f78774

fix for SW clustering

c4922cb

dasphy and others added 2 commits December 13, 2023 23:16

Merge branch 'HEP-FCC:main' into tli-clustering-new-readout

35aa9fd

update with k4geo for SW FCCee

5cc6d81

BrieucF mentioned this pull request Dec 14, 2023

update sliding window clustering FCCee with k4geo BrieucF/LAr_scripts#18

Merged

fix

424bfba

dasphy and others added 2 commits January 24, 2024 17:03

Merge branch 'HEP-FCC:main' into tli-clustering-new-readout

c5bd49b

mv struct cluster

f7189dd

kjvbrt reviewed Jan 24, 2024

View reviewed changes

RecFCCeeCalorimeter/src/components/CaloTowerToolFCCee.h Outdated Show resolved Hide resolved

Tong Li added 2 commits January 24, 2024 18:08

rm towersNumber()

a75a217

update

c7430bb

kjvbrt merged commit 6740f21 into HEP-FCC:main Feb 9, 2024
0 of 3 checks passed

dasphy mentioned this pull request Feb 12, 2024

update sliding window clustering job options HEP-FCC/FCC-config#140

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

update sliding-window clustering for Theta-Module readout #58

update sliding-window clustering for Theta-Module readout #58

dasphy commented Nov 8, 2023 •

edited

Loading

BrieucF commented Nov 14, 2023

BrieucF Nov 14, 2023

kjvbrt Nov 14, 2023

dasphy Nov 14, 2023

giovannimarchiori Nov 30, 2023

dasphy Dec 1, 2023

kjvbrt commented Nov 14, 2023 •

edited

Loading

dasphy commented Nov 14, 2023

dasphy commented Dec 10, 2023

kjvbrt commented Dec 11, 2023

dasphy commented Dec 11, 2023 •

edited

Loading

dasphy commented Dec 12, 2023

kjvbrt commented Dec 12, 2023 •

edited

Loading

dasphy commented Dec 13, 2023

dasphy commented Dec 13, 2023

dasphy commented Dec 13, 2023

dasphy commented Dec 14, 2023 •

edited

Loading

dasphy commented Dec 22, 2023

kjvbrt commented Jan 24, 2024

dasphy commented Jan 24, 2024

dasphy commented Feb 1, 2024

update sliding-window clustering for Theta-Module readout #58

update sliding-window clustering for Theta-Module readout #58

Conversation

dasphy commented Nov 8, 2023 • edited Loading

BrieucF commented Nov 14, 2023

BrieucF Nov 14, 2023

Choose a reason for hiding this comment

kjvbrt Nov 14, 2023

Choose a reason for hiding this comment

dasphy Nov 14, 2023

Choose a reason for hiding this comment

giovannimarchiori Nov 30, 2023

Choose a reason for hiding this comment

dasphy Dec 1, 2023

Choose a reason for hiding this comment

kjvbrt commented Nov 14, 2023 • edited Loading

dasphy commented Nov 14, 2023

dasphy commented Dec 10, 2023

kjvbrt commented Dec 11, 2023

dasphy commented Dec 11, 2023 • edited Loading

dasphy commented Dec 12, 2023

kjvbrt commented Dec 12, 2023 • edited Loading

dasphy commented Dec 13, 2023

dasphy commented Dec 13, 2023

dasphy commented Dec 13, 2023

dasphy commented Dec 14, 2023 • edited Loading

dasphy commented Dec 22, 2023

kjvbrt commented Jan 24, 2024

dasphy commented Jan 24, 2024

dasphy commented Feb 1, 2024

dasphy commented Nov 8, 2023 •

edited

Loading

kjvbrt commented Nov 14, 2023 •

edited

Loading

dasphy commented Dec 11, 2023 •

edited

Loading

kjvbrt commented Dec 12, 2023 •

edited

Loading

dasphy commented Dec 14, 2023 •

edited

Loading