PandoraPFA / LArContent

Algorithms and tools for LAr TPC event reconstruction
GNU General Public License v3.0
6 stars 48 forks source link

Counting of neutrino final state Pfos #23

Closed a-d-smith closed 6 years ago

a-d-smith commented 6 years ago

I think this function is counting neutrino final state PFOs multiple times. https://github.com/PandoraPFA/LArContent/blob/d3eb30c6e087257b83acee451cc368d6d2ce819f/larpandoracontent/LArHelpers/LArMonitoringHelper.cc#L32-L39

LArPfoHelper::IsFinalState already accounts for neutrino final states https://github.com/PandoraPFA/LArContent/blob/d3eb30c6e087257b83acee451cc368d6d2ce819f/larpandoracontent/LArHelpers/LArPfoHelper.cc#L336-L345

StevenGreen1 commented 6 years ago

Hi Andy,

I was just having a quick look at this issue that you highlighted. I think I can see the issue if you have both neutrinos and daughters of neutrinos in the inputPfoList in LArMonitoringHelper.

When looping over inputPfoList:

  1. When encountering a neutrino in inputPfoList the daughters will be added.

  2. When encountering the daughters in inputPfoList they will also be added.

Having done a quick search for uses of this function in LArPandoraContent I can see that this function if called only if m_primaryPfosOnly is set to true. I assume this would prevent double counting as the neutrinos themselves wouldn't be included in the inputList (@johnmarshall80 )?

Is that this issue you were thinking of Andy?

Cheers

Steve

PandoraPFA commented 6 years ago

I just looked into one event, and it would appear that there is indeed double counting, but it seems that some things are "rescued" downstream, because the containers that matter are of type std::unordered_map, keyed upon pandora::ParticleFlowObject *, so uniqueness becomes enforced at this point.

A couple of the functions called EventValidationAlgorithm::GetPfoIdMap andLArMonitoringHelper::GetPfoToCaloHitMatches do wastefully calculate the same information twice.

This would have been spotted if the return type, upon insertion to the map hadn't been cast to void in EventValidationAlgorithm

I suggest that this is fixed as part of the current remastering and our ongoing pull request!

Input pfo, 0x80cc420, id 14, nParents 0, nDaughters 3, nClusters 0, nVertices 1, finalState 0, neutrino 1
--Add 3 daughter pfos
-----Daughter 0x781b360, id 13, nClusters 4, nVertices 1, finalState 1, neutrino 0
-----Daughter 0x781aaa0, id 13, nClusters 4, nVertices 1, finalState 1, neutrino 0
-----Daughter 0x775f2f0, id 13, nClusters 4, nVertices 1, finalState 1, neutrino 0
Input pfo, 0x781b360, id 13, nParents 1, nDaughters 0, nClusters 4, nVertices 1, finalState 1, neutrino 0
--Add this pfo 0x781b360
Input pfo, 0x781aaa0, id 13, nParents 1, nDaughters 0, nClusters 4, nVertices 1, finalState 1, neutrino 0
--Add this pfo 0x781aaa0
Input pfo, 0x775f2f0, id 13, nParents 1, nDaughters 0, nClusters 4, nVertices 1, finalState 1, neutrino 0
--Add this pfo 0x775f2f0
End ExtractTargetPfos output size 6
Target pfo 0x781b360, id 13, nClusters 4, nVertices 1, finalState 1, neutrino 0
Target pfo 0x781aaa0, id 13, nClusters 4, nVertices 1, finalState 1, neutrino 0
Target pfo 0x775f2f0, id 13, nClusters 4, nVertices 1, finalState 1, neutrino 0
Target pfo 0x781b360, id 13, nClusters 4, nVertices 1, finalState 1, neutrino 0
Target pfo 0x781aaa0, id 13, nClusters 4, nVertices 1, finalState 1, neutrino 0
Target pfo 0x775f2f0, id 13, nClusters 4, nVertices 1, finalState 1, neutrino 0
Target pfo 0x775f2f0, id 13, nClusters 4, nVertices 1, finalState 1, neutrino 0, inAlg id# 4
Target pfo 0x781aaa0, id 13, nClusters 4, nVertices 1, finalState 1, neutrino 0, inAlg id# 2
Target pfo 0x781b360, id 13, nClusters 4, nVertices 1, finalState 1, neutrino 0, inAlg id# 0
Target pfo 0x775f2f0, id 13, nClusters 4, nVertices 1, finalState 1, neutrino 0, nHits 376
Target pfo 0x781aaa0, id 13, nClusters 4, nVertices 1, finalState 1, neutrino 0, nHits 764
Target pfo 0x781b360, id 13, nClusters 4, nVertices 1, finalState 1, neutrino 0, nHits 1978

Still digging into some details.

PandoraPFA commented 6 years ago

After some worrying about why we had never seen even a whisper of this issue before, I traced this to the simple fact that we have only ever provided reco neutrinos in the input list for our validation work.

In all use cases, we have m_useRecoNeutrinosOnly set to true, so the target collection logic is tweaked as per: LArMonitoringHelper::ExtractTargetPfos(m_useRecoNeutrinosOnly ? recoNeutrinoList : allRecoParticleList, m_primaryPfosOnly, pfoList);

The flow in my test event is then as shown below:

Target pfo 0x4b6fe70, id 14, nClusters 0, nVertices 1, finalState 0, neutrino 1
---Add 3 daughter pfos                                                         
------Daughter pfo 0x3923500, id 13, nClusters 4, nVertices 1, finalState 1, neutrino 0
------Daughter pfo 0x4b720e0, id 13, nClusters 4, nVertices 1, finalState 1, neutrino 0
------Daughter pfo 0x5171fe0, id 13, nClusters 4, nVertices 1, finalState 1, neutrino 0
End ExtractTargetPfos output size 3

So everything that's gone previously is OK; now we can move forwards! Phew!