Hi @cgrefe,
Yes thanks, I'd suggest that. Indeed the only requirement for PROC accepting a FT0-violating MR is to discuss it and approve it in the Reconstruction meeting (see FT0 Policy), but it's great to see you've done additional steps at the request of PC, so hopefully the approval should go smoothly. You can just discuss it verbally during the PROC report, or present any slides on details if you'd prefer.
Hi @cgrefe, that's great to hear. The next step would be to have a representative of the MR join the next Reconstruction meeting (Tuesday at 5pm) to briefly discuss the details of this FT0V change so it can be approved by PROC. Would you or someone be available?
Changes look sensible enough & were heavily validated. @jojungge can you confirm that the fixes to the strip positions of the BIS are the only changes expected to cause output differences?
No I believe we're still requiring offline-sw-review-approved for all ref-changing MRs. 24.0 requires it from PROC after discussion at Reco meeting, main just requires it from reco or software coordinators. Pinging @christos, @jcatmore , @nstyles , @jchapman if they want to correct this.
Thanks @pscholer for the feedback, apologies that the Reco meeting time was conflicting. The spirit of this MR was discussed, and the general agreement of Reco, Software, & Data Prep is to avoid FT0 violating MRs in 24.0 for such short-term bandages. The preference is to continue to use long postExecs until the global tags are ready.
It was agreed that, in spirit, removing the hard-coded paths permanently causes less confusion and would be acceptable as a FT0V MR to 24.0 at this moment in time. I think that if you agree with this plan, we can revisit with the updated MR whether it needs additional discussion with experts and PROC at the reco meeting as per policy.
This was discussed at today's Reco meeting. Consensus is that the MET-related bug-fix would be acceptable for 24.0 branch if it is split from the other new developments. Mark will run validation plots and get approval from JetETMiss (don't expect issues with MET WPs given previous approval to switch from delta-R to linking for MET)
@pscholer shall we discuss this at today's Reco meeting?
Jeff Dandoy (40095ec0) at 07 Mar 11:58
test_mc21_13p6TeV_hi_withtrigger.sh bump ART to 24.0
... and 617 more commits
For Reco-changing outputs, our strategy is to approve the changes in the red CI as reco coordinators, ask the L1 shifters to then continue with the review (ignoring the red CI), and have the day's Release Coordinator perform the CI reference update after L1 approval is secured.
I don't know if Derivation changes want to also follow this strategy, but with approval for proceeding with a red CI from someone else. It looks like the ftag conveners have effectively approved this, and analysis-review-approved was given.
This was discussed and approved at Reco meeting as Duc noted, and can have the label added by PROC. However, I'd wait until the issues you mentioned are solved & @jojungge resolves all threads.
This FT0V MR was mentioned in yesterday's Reco meeting, but no decision was attempted as we understood the decision on including this in 24.0 vs main is being discussed by PC & CP groups. Was this discussed at Monday's PC meeting, or otherwise what is the plan?
For workflow simplicity and reducing mistakes, we had changed the general workflow for Reco-output-changing MR's some time ago so that only Release Coordinators updated the CI references, after L1 approval, using automated tools. Does this not apply for FT0V MRs in 24.0? Pinging @nstyles @jchapman @jcatmore
Hi @jbeacham, The changes to the CI are expected and the code review can proceed as normal. The references will be auto-updated by the RC before merging.
Looks good, and thanks for the clarifications & info. These tests were switched to python -m after discussion at the CA hackathon, but I'll revisit this based on your guidance.
This was noted by Johannes and Peter to be an essential bugfix that should target 24.0 for upcoming data. I'd give it the offline-sw-review-approved label but this is FT0V, I imagine @dta @ahabouel should give the approval for the L1 shifter to continue the review? Then RC can update the reference while merging.