Project

General

Profile

Bug #7051

Bug #6394: Verify that association query objects are created outside tight loops

FindManyP() usage in Calorimetry module

Added by Gianluca Petrillo over 6 years ago. Updated over 4 years ago.

Status:
Closed
Priority:
Normal
Category:
Reconstruction
Target version:
-
Start date:
09/22/2014
Due date:
10/24/2014
% Done:

100%

Estimated time:
3.00 h
Occurs In:
Experiment:
LArSoft
Co-Assignees:
Duration: 33

Description

Calorimetry module shows dubious practises in Calorimetry::produce().

larana/Calorimetry/Calorimetry_module.cc

The action I recommend:
- since you use it, #include "art/Framework/Core/FindManyP.h"
- bring the queries FindManyP() out of the loops
- use const std::vector<art::Ptr<T>>& to store the result of FindManyP::at(), that returns a constant reference

Contact person: Wesley Ketchum ().

History

#1 Updated by Wesley Ketchum over 6 years ago

  • Due date set to 10/14/2014

Will do. I'll try to have this done by next week.

#2 Updated by Wesley Ketchum over 6 years ago

  • Due date changed from 10/14/2014 to 10/21/2014
  • % Done changed from 0 to 10
  • Estimated time changed from 1.00 h to 3.00 h

I've looked at the code, and there is some strangeness going on I'm not sure of. One thing: FindManyP gets space points associated with hits in a loop over each plane (which seems ok), but then loops over those space points, and calls it again to get the hits associated to each space point (which seems totally unnecessary), and of course only looks at the hits on the plane in question (so, seems super-totally unnecessary). It seems FindManyP is the completely wrong tool to use here---we really should be looking at the std::pair of art ptrs, I think---but I need a better understanding of the code to know how best to fix it.

So, I'm including Eric and Tingjun as watchers here, and pushing the date to have it done to next week (when Tingjun should be back from vacation).

#3 Updated by Wesley Ketchum over 6 years ago

  • Due date changed from 10/21/2014 to 10/24/2014
  • % Done changed from 10 to 30

Just talked to Tingjun, and we've agreed on a plan for getting rid of the worst offenders for FindManyP in a loop. Should be done relatively soon.

#4 Updated by Tingjun Yang over 4 years ago

  • Status changed from Assigned to Resolved

Resolved a while ago.

#5 Updated by Tingjun Yang over 4 years ago

  • Status changed from Resolved to Closed
  • % Done changed from 30 to 100

Also available in: Atom PDF