eic / EICrecon

EIC Reconstruction - JANA based
https://eic.github.io/EICrecon
GNU Lesser General Public License v3.0
6 stars 28 forks source link

Imaging clusters missing in reconstructed files #351

Closed mariakzurek closed 1 year ago

mariakzurek commented 1 year 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. Check the newest produced test files in Actions, for example here: https://github.com/eic/EICrecon/actions/runs/3450751526

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

Expected clusters from imaging layers

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

No clusters from imaging layers reconstructed

I've reviewed again the parameters in reco_flags.py

and I've spotted one issue. The min calo energy is 0.5 MeV and it should be 0.5 MeV/sampling_fraction, as we discussed here: https://github.com/eic/EICrecon/pull/253#discussion_r1001288982

I don't think that this will cause all clusters to disappear, because it's a lower limit, but it should be fixed anyhow.

The rest of the parameters look ok

('BEMC:EcalBarrelImagingProtoClusters:input_tag', 'EcalBarrelImagingRecHits', 'Name of input collection to use'), ('BEMC:EcalBarrelImagingProtoClusters::localDistXY', '2.0,2.0', '* [mm]'), ('BEMC:EcalBarrelImagingProtoClusters::layerDistEtaPhi', '0.01,0.01', '* [radian]'), ('BEMC:EcalBarrelImagingProtoClusters::neighbourLayersRange', '2.0', ''), ('BEMC:EcalBarrelImagingProtoClusters::sectorDist', '3.0*cm', ''), ('BEMC:EcalBarrelImagingProtoClusters::minClusterHitEdep', '0.', ''), ('BEMC:EcalBarrelImagingProtoClusters::minClusterCenterEdep', '0.', ''), ('BEMC:EcalBarrelImagingProtoClusters::minClusterEdep', '0.5*MeV', ''), ('BEMC:EcalBarrelImagingProtoClusters::minClusterNhits', '5', ''),

But are they really passed with the proper units? Is, e.g., localDistXY passed as 2*mm or as 2? or is 0.5*MeV passed as 0.5 (GeV?)?

faustus123 commented 1 year ago

I will take a look at this later today or this evening.

faustus123 commented 1 year ago

PR #354 now fixes this issue. I assigned @mariakzurek to review it.