Bug #7051
Bug #6394: Verify that association query objects are created outside tight loops
FindManyP() usage in Calorimetry module
100%
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 (wketchum@fnal.gov).
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