cms-sw
BPHNano PR review
BPHNano is now in production. We need to make sure that changes to the following packages are signed off by BPH PAG experts:
- PhysicsTools/BPHNano
- PhysicsTools/NanoAOD/python/custom_bph_cff.py
Is it possible?
FYI: @gmelachr
A new Issue was created by @drkovalskyi.
@Dr15Jones, @antoniovilela, @makortel, @mandrenguyen, @rappoccio, @sextonkennedy, @smuzaffar can you please review it and eventually sign/assign? Thanks.
cms-bot commands are listed here
Setting up watchers for those two packages is straightforward (i.e. designated people get GitHub notification for all PRs touching those packages).
Requiring PAG to sign all PRs touching those packages is technically straightforward too, but we do not have precedent on PAGs (or POGs or DPGs) signing PRs (unless such signatory is explicitly requested on a given PR by someone), so that route might need some discussion first. The present mode of operation is that the "L2 area" responsible for the packages (@cms-sw/xpog-l2 in this case) consults the PAGs/POGs/DPGs.
I think we have an exceptional case here. The BPHNano package contains non-trivial reconstruction and fitting code, which is used by several analyzers. I'm sure xPOG can handle the technical side of the review, but we need to ensure that the changes won't affect the physics side and that’s probably too much to ask from xPOG. I suppose we normally rely on RelVals and unit tests to detect problems in general, right?
@hqucms, @ftorrresd what do you think?
Generally physics content is checked in PR tests and in PdmV's release validation (at every prerelease). Although I don't know to what extent NanoAOD physics content is checked in PR tests (maybe @cms-sw/xpog-l2 could clarify).
Physics-checking unit tests would be lovely, but rarely implemented in practice.
Adding L1s @sextonkennedy @srimanob @stlammel and @cms-sw/orp-l2 in case they have any input.
(Just to be clear, I don't object adding a GitHub team for BPH PAG and requiring them to sign PhysicsTools/BPHNano and PhysicsTools/NanoAOD if they and @cms-sw/xpog-l2 agree. But because this approach would deviate from our established practice and set a precedent, I think we should spend time first to understand if an approach within our established practice would be sufficient or not)
Thanks, Matti. I'm also not insisting on a specific way to handle it but rather looking for a way to handle it properly. It's probably something a bit new compared to what was done up until now, since we effectively do physics analysis during NanoAOD production, not just produce objects used for analysis as is done in all other POGs and PAGs. It's a BPH-specific issue, because we need to reconstruct multiple final states using kinematic fits requiring expertise.
