Add support for reconstruction of "turbine" endcap ecal

[varnes]

This PR is a first pass at including the "turbine" ecal endcap into reconstruction. At the moment only the sliding windows algorithm for creating clusters is supported. In addition to adding code to support this geometry, there was also a change in CaloTowerToolFCCee::CellsIntoTowers, where segmentation->theta(cell.getCellID()) was changed to cellTheta (which was already calculated from the cell position). This change means that the segmentation is no longer required by the CellsIntoTowers method, so it was removed from the list of passed parameters.

Further details about the turbine geometry and the segmentation used for its reconstruction can be found at https://indico.cern.ch/event/1298458/contributions/5977217/attachments/2875296/5035181/FCC_week_June_2024.pdf

This PR is intended to be in concert with a similar PR for k4geo.

[kjvbrt]

Hi @varnes, can you add link to the k4geo PR?

[varnes]

Hi @kjvbrt ,

The corresponding k4geo PR is key4hep/k4geo#347.

Best regards,
Erich

[giovannimarchiori]

Hi @varnes ,

thanks a lot for this PR!

Overall it looks good to me, but I think that now it is quite evident from the code that we don't need anymore the tool to have all the members of type dd4hep::DDSegmentation::Segmentation* and of type SegmentationType, because the only place where the segmentations are used is in the function that retrieves the phi and theta extremes of the readout in CaloTowerToolFCCee::retrievePhiThetaExtrema.
I think you can remove all those members, remove the code that retrieves the segmentation in initialize(), and modify retrievePhiThetaExtrema to accept the name of the readout rather than segmentation pointer + type. You can retrieve (temporarily) the segmentation in retrievePhiThetaExtrema.
And basically all code of towersNumbers except the last two lines could go into initialize() to calculate at the beginning of the alg the max theta and phi

If that is clear and you want to do it in this PR, that'd be great for me, otherwise we can postpone it to a separate PR.
Pinging also @BrieucF

[varnes]

Hi @giovannimarchiori ,

I'm happy in principle with removing the members that are no longer needed, and making the related changes you suggest. However between ICHEP and some vacation, I will not have many free cycles between now and 1 August.

-- Erich

[varnes]

Hi @giovannimarchiori ,

The segmentation members have been removed, and the other changes you suggested are now implemented. There does still need to be a check that the segmentations are read in correctly (and therefore that phimax and thetamax are set correctly), so I added a set of bool members to keep track of that.

[giovannimarchiori]

HI @varnes
thanks a lot for these updates.
There are 2-3 little things to be fixed (one indentation and passing directly a std::string to the CaloTowerToolFCCee::retrievePhiThetaExtrema method). We could also get rid of the booleans m_xxxSegmentationOK and return a StatusCode::Failure when the segmentation is not OK, but we can do it also in a separate PR if you don't have time. I also think I'll have a further shot at this code in a later PR to make it agnostic about the number of calorimeter sub-detectors, so the boolean removal could also go in there.

[giovannimarchiori]

note for myself: we might want to refactor this tool in a later PR so that it accepts a list of cell collections and a list of readout names, making it more agnostic of the number of calorimeter sub detectors...
Then this hardcoded 7 numbers would be the length of the list

[giovannimarchiori]

Hi @varnes thanks a lot, the PR looks good to me now

[giovannimarchiori]

Hi @varnes, we merged a PR to fix some tests that were failing due to the EventCounter, could you please rebase your branch (and fix conflicts in case)? Hopefully then all tests will pass and we can merge it.
Thanks!

[varnes]

Hi @giovannimarchiori ,

I think it should be OK now...

[giovannimarchiori]

Hi @varnes ,

as you can see from the error messages above, the tests for the nightly builds are failing. If you click on Details you will see that there are 3 tests failing:

• FCCeeLAr_simRecoAllegroV3:

File "RecFCCeeCalorimeter/tests/options/run_thetamodulemerged.py", line 503, in <module>
    fileName=os.environ['FCCBASEDIR'] +
  File "/cvmfs/sw-nightlies.hsf.org/key4hep/releases/2024-07-24/x86_64-almalinux9-gcc11.4.1-opt/python/3.10.14-a23y4f/lib/python3.10/os.py", line 680, in __getitem__
    raise KeyError(key) from None
KeyError: 'FCCBASEDIR'

=> if you want topoclustering to be run in this test, can you please use the following files
neighboursMap = "https://fccsw.web.cern.ch/fccsw/filesForSimDigiReco/ALLEGRO/ALLEGRO_o1_v03/neighbours_map_ecalB_thetamodulemerged.root"
noiseMap = "https://fccsw.web.cern.ch/fccsw/filesForSimDigiReco/ALLEGRO/ALLEGRO_o1_v03/cellNoise_map_electronicsNoiseLevel_ecalB_thetamodulemerged.root"

so that FCCBASEDIR is not needed?

• FCCeeLAr_benchmarkCorrection:

File "RecFCCeeCalorimeter/tests/options/run_thetamodulemerged_SW_benchmarkCorr.py", line 402
    cellPositionEcalEndcapTool = CellPositionsECalEndcapTurbineSegTool(
IndentationError: unexpected indent

=> there is a spurious space at the beginning of line 402 in this file, can you please fix it?

• FCCeeLAr_runxtalk .

*** Break *** segmentation violation



===========================================================
There was a crash.
This is the entire stack trace of all threads:
===========================================================

#5  <signal handler called>
#6  0x00007f85161f8935 in CaloTowerToolFCCee::CellsIntoTowers(std::vector<std::vector<float, std::allocator<float> >, std::allocator<std::vector<float, std::allocator<float> > > >&, edm4hep::CalorimeterHitCollection const*, bool) () from /k4reccalorimeter/build/RecFCCeeCalorimeter/libk4RecFCCeeCalorimeterPlugins.so

I think I had fixed this error in the code I sent you by mail. I have the impression that you put the file runEcalBarrel_ReconstructionTopoClusters_crosstalk.py in the wrong directory - you added it as a new test to RecFCCeeCalorimeter/tests/options/ while it should have been in RecCalorimeter/tests/options/

Please make these 3 changes and push, and check if the tests in the nightlies pass successfully as they should.

@BrieucF : if there are still errors after the next attempt, it might be more efficient if we merge the PR and I fix the tests in a separate PR.

[varnes]

Hi @giovannimarchiori ,

Would it be best for me to just update these scripts to the versions that are currently in the main branch, or should I take the ones from your email?

Thanks,
Erich

[giovannimarchiori]

Hi @varnes ,

it was quicker for me to merge your PR and fix the remaining issues in #106

Thanks, cheers,
Giovanni