Project

General

Profile

Actions

Bug #1411

closed

Run out of some TClonesArray's bounds in CbmKFTrackQA with CbmMatchRecoToMC

Added by Oleg Golosov over 2 years ago. Updated over 2 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Target version:
Start date:
10/22/2019
Due date:
10/28/2019
% Done:

100%

Estimated time:
10.00 h
Spent time:

Description

Running CbmKFTrackQA task along with CbmMatchRecoToMC task (example macro is attached) produces lots of errors like the following one:

Error in <TClonesArray::At>: index 164 out of bounds (size: 100, this: 0xf3c8ac0)

The size of the TClonesArray is always 100.

Disabling CbmMatchRecoToMC silences the errors.
CbmMatchRecoToMC task itself does not produce such an error either.

The error is not observed with APR19 release and CbmKFTrackQA was not changed since then so it is most probably connected to the changes in CbmMatchRecoToMC.
Relevant for both GEANT3 and GEANT4.


Files

run_KFtrackQA.C (5.92 KB) run_KFtrackQA.C Oleg Golosov, 10/18/2019 09:35 PM

Related issues

Related to CbmRoot - Development #1422: Consistent naming of TOF data branchesClosedVolker Friese11/25/201911/29/2019

Actions
Actions #1

Updated by Volker Friese over 2 years ago

  • Status changed from New to Assigned
  • Start date changed from 10/18/2019 to 10/22/2019
  • Estimated time set to 10.00 h
Actions #2

Updated by Volker Friese over 2 years ago

  • Status changed from Assigned to In Progress

I confirm the observation, both in OCT19 and in trunk (r15161).

Actions #3

Updated by Volker Friese over 2 years ago

  • Status changed from In Progress to Resolved
  • Assignee changed from Volker Friese to Oleg Golosov
  • % Done changed from 0 to 100

The problem is traced to the digi matches of TOF. The situation is rather confusing. There is a branch TofDigiMatch in the raw file and one with the same name in the rec file. The latter seems to link TofHits to TofDigis. I do not know what happens if there are two branches with the same name in the input tree, but probably rather unwanted behaviour.

I changes the name of the branch created by the TOF Hit Finder (TofSimpleClusterizer) and adjusted CbmMatchRecoToMC accordingly. The error vanished for me. Please check. Done both in OCT19 and trunk.

Actions #4

Updated by Pierre-Alain Loizeau over 2 years ago

That's strange, from what I remember and see in the documentation I made at the time, it should be unique. The name was kept generic due to compatibility with analysis classes which are probably not in use anymore.

Could it be that something else creates its own branch with the same name?
Could it also be that CbmKFTrackQA somehow uses the match arrays differently from other classes? (maybe with the wrong assumptions about what is contained in the match array?)

Could you update https://redmine.cbm.gsi.de/projects/cbmroot/wiki/Using_MC-Hit_matches_for_TOF to reflect the changes you did then?
I think we will have to scan some of the QA and analysis classes in the tof folder to update them accordingly in order to be consistent everywhere (for example between simulation and real data)
Suspects (from grep -rn "TofDigiMatch" tof/* ):

tof/TofDigi/CbmTofDigitizerBDF.cxx
tof/TofDigi/CbmTofDigitize.cxx
tof/TofReco/CbmTofEventClusterizer.cxx
tof/TofReco/CbmTofCosmicClusterizer.cxx
tof/TofReco/CbmTofTestBeamClusterizer.cxx
tof/TofTests/CbmTofAnaTestbeam.cxx
tof/TofReco/CbmTofBuildDigiEvents.cxx
tof/TofQA/CbmTofHitFinderQa.cxx

PS: for all possible impacted classes

grep --exclude-dir=build -rn "TofDigiMatch" *

Actions #5

Updated by Volker Friese over 2 years ago

The situation was (before r15219):
CbmTofDigitize creates a branch TofDigiMatch. This is used by default in CbmDigitize.
CbmTofDigitizeBDF creates a branch TofDigiMatchPoints.

CbmTofSimpClusterizer creates a branch TofDigiMatch (links hits to digis).

So, the conflict is obvious. I resolved this by renaming the branch created in the clusterizer to TofHitDigiMatch. I did not touch CbmTofDigitizeBDF yet.

The common understanding / definition is that matches always connect data classes with their proper MC reference, which is MCPoint for digis, clusters and hits, MCTracks for tracks, and MCEvents for events. We should stick to that in order to avoid confusion.

TOF introduces another match branch to link hits to digis. In all other detectors, this information is stored with the hit or cluster class itself. Formally, of course, the match class provides the proper functionality to store this information, so why shoud it not be used.. but then, if would be better to make the hit-digi match a member of the hit class and not to store it in a different branch.

We have different branches for the (real) matches just for the reason that the data look the same when coming from simulation or from real data. In the latter case, the match branches are just not present. For linking between raw and reco classes, this argument does not hold.

Actions #6

Updated by Oleg Golosov over 2 years ago

  • Assignee changed from Oleg Golosov to Volker Friese

I confirm that the error disappeared.

Actions #7

Updated by Pierre-Alain Loizeau over 2 years ago

Added Norbert as watcher, as he is the one having the most experience on how the TOF clusterizer and its ouput are used right now.

Also putting the issue from resolved to feedback as I fear we may have fixed the simulation by breaking the real data analysis. Most probably the fix is as simple as just changing the branch name in all accesses, but we should not forget to do it everywhere right now otherwise strange problems may pop up when we merge back beamtime analysis (e.g mcbm) into trunk.

Actions #8

Updated by Pierre-Alain Loizeau over 2 years ago

  • Status changed from Resolved to Feedback
Actions #9

Updated by Volker Friese over 2 years ago

Now, what was fixed was not the simulation - I did not interfere with any digitizer, just with TofSimpClusterizer.

You are right, all the other clusterizers also create a branch TofDigiMatch for connecting hits to digis. So, all these are not compatible with CbmTofDigitizer.

I think a consistent naming scheme should be introduced throughout, and it would be nice if that would be in line with conventions for other detectors.

It is easy to change the branch names in the tof directory. What you have in other places I do not know. For me, having separate classes for each data taking signals that we are quite far from maturity (or maintanability) still.

Let us briefly discuss in tomorrow's software meeting.

Actions #10

Updated by Volker Friese over 2 years ago

Actions #11

Updated by Volker Friese over 2 years ago

  • Due date set to 10/28/2019
  • Status changed from Feedback to Closed
Discussion result:
  • Keep the solution as is for OCT19.
  • Follow-up issue to arrive at a consistent naming for TOF mtch branches: #1422.

This issue can thus be closed.

Actions

Also available in: Atom PDF