Feature/howard v09370103p01 nuid features #236
Conversation
…ject in SRSlice instead of all directly SRSlice
…tend this to a few other fills too but for now this is what I did.
brucehoward-physics
requested review from
cjbacchus,
FernandaPsihas,
PetrilloAtWork and
henrylay97
sbncode/CAFMaker/FillReco.cxx
Outdated
| //...................................................................... | ||
| template<class T, class U> | ||
| void CopyPropertyIfSet( const std::map<std::string, T> map, const std::string search, U &value ) | ||
| { | ||
| if (map.count(search)) value = map.at(search); | ||
| } |
There was a problem hiding this comment.
search should definitely be a const reference. I think map (bad name because it clashes with the type, maybe props?) should be too, but I think you will need to change the implementation to not use at(). You can use find() to get an iterator and dereference that.
Something like this
| //...................................................................... | |
| template<class T, class U> | |
| void CopyPropertyIfSet( const std::map<std::string, T> map, const std::string search, U &value ) | |
| { | |
| if (map.count(search)) value = map.at(search); | |
| } | |
| //...................................................................... | |
| template<class T, class U> | |
| void CopyPropertyIfSet( const std::map<std::string, T>& props, const std::string& search, U& value ) | |
| { | |
| auto it = props.find(search); | |
| if(it != props.end()) value = *it; | |
| } |
There was a problem hiding this comment.
The map iterator returns a pair, so I think I want it->second instead of *it right? Update to method and removing the include noted below now committed. Does this seem better?
There was a problem hiding this comment.
Yep, this is what happens when I freestyle code... Looks good now, thanks.
sbncode/CAFMaker/FillReco.h
Outdated
| @@ -35,6 +35,7 @@ | |||
| #include "nusimdata/SimulationBase/MCTruth.h" | |||
|
|
|||
| #include "sbnanaobj/StandardRecord/SRSlice.h" | |||
| #include "sbnanaobj/StandardRecord/SRNuID.h" | |||
There was a problem hiding this comment.
Do you need this include?
There was a problem hiding this comment.
I guess SRSlice.h should bring in NuID.h in any case. I'll remove when I test the above suggestion on an event.
…h and change the CopyPropertyIfSet function a bit.
sbncode/CAFMaker/FillReco.cxx
Outdated
| srslice.nuid.nufspfos = -9999.f; | ||
| srslice.nuid.nutothits = -1; | ||
| srslice.nuid.nuvtxy = -9999.f; | ||
| srslice.nuid.nuwgtdirz = -9999.f; | ||
| srslice.nuid.nusps = -9999.f; | ||
| srslice.nuid.nueigen = -9999.f; | ||
| srslice.nuid.crlongtrkdiry =-9999.f; | ||
| srslice.nuid.crlongtrkdef = -9999.f; | ||
| srslice.nuid.crlongtrkhitfrac = -9999.f; | ||
| srslice.nuid.crmaxhits = -1; |
There was a problem hiding this comment.
Isn't this what srslice.nuid.setDefault() is for?
There was a problem hiding this comment.
Yeah I suppose I could just run setDefault() here 😅 I can make this change when I make the Standard Record changes.
…, use setDefault(). Reponse to PR 236
Updated PR that uses v09_37 series as the base. Requires changes in larpandoracontent to be useful.
@cjbackhouse is this basically what you had in mind for the helper function?