Refactor reco2 workflow, and promote standard_reco2_sbnd.fcl back to the main workflow again #528
Conversation
No additional drops are currently needed for reco2, but add the infrastructure now anyway...
absolution1
requested review from
yangtj207,
marcodeltutto,
bear-is-asleep,
henrylay97 and
lynnt20
There was a problem hiding this comment.
Looks great, thanks @absolution1!
One thing we should probably pay attention to is to make sure that all our fcls end with _sbnd.fcl to make sure there's no conflict if there's a similar fcl in sbncode or even icaruscode in case people set up icaruscode in parallel.
There was a problem hiding this comment.
Looks good to me! Thank you so much for doing this work @absolution1!
| , fmatcharaSCE | ||
| , fmatchoparaSCE | ||
| , opt0finderSCE | ||
| ] |
There was a problem hiding this comment.
I'm just going to mention here that we've often talked about moving a lot of the producers currently run in the CAF stage into reco2 (CRUMBS, stubs, the MCS tool, the PID BDTs etc).
This shouldn't be part of this PR as this is meant to reorganise whilst maintaining behaviour. But now that this workflow fcl is nice & clean, we should look at doing this soon.
| # FIXME The following services are temporarily included here to test tht the workflow is unchanged | ||
| LArFFT: @local::sbnd_larfft | ||
| SignalShapingServiceSBND: @local::sbnd_signalshapingservice | ||
| BackTrackerService: @local::sbnd_backtrackerservice | ||
| ParticleInventoryService: @local::sbnd_particleinventoryservice |
There was a problem hiding this comment.
And the longer term plan would be to move these into a table?
There was a problem hiding this comment.
As with the reco1 changes, the name should be reco2_comm.fcl
| @@ -1,8 +1,11 @@ | |||
| # SBND Core FHiCL Files | |||
|
|
|||
| 4th October 2024 (Dom Brailsford) | |||
| `standard_reco2_sbnd.fcl` has been promoted back to an up-to-date fcl and has rejoined the standard workflow | |||
There was a problem hiding this comment.
Might be good to explicitly mention that sce is now included in standard_reco2_sbnd.fcl for completion/documentation sake here!
| outputs.out1.dataTier: "reconstructed" | ||
| outputs.out1.outputCommands: [@sequence::outputs.out1.outputCommands, @sequence::sbnd_reco2_drops] | ||
|
|
||
| #FIXME permanently enable SCE in a service rather than override specific params in this way... |
There was a problem hiding this comment.
maybe we can delete this #FIXME line now since it looks like enabling SCE is through a service?
There was a problem hiding this comment.
I think Dom just means make it the default right? Rather than overrides at the bottom?
There was a problem hiding this comment.
thank you @absolution1!! this looks great 😄 just left some very minor comments!
Description
This PR refactors reco2, reusing the fcl organisation defined in PR: #460
This PR
standard_reco2_sbnd.fclback to the main workflowreco2_sce.fclentirely and updates any dependencies to point tostandard_reco2_sbnd.fclThe original and reorganised fcls have been diff'd to check nothing has substantially changed. There is a minor change in the number of dropped products (more are dropped now). These are earlier stage products already dropped by reco1, so this is a benign change. All other diffs are fcl-bookkeeping that should not affect the output (e.g. sequence names changing from
reco2_scetoreco2. I have attached the diffs, for completeness.I've picked an assortment of reviewers, I'm not anticipating/expecting everyone actually looks.
Checklist
Reviewers,AssigneesDevelopementreco2_data_diff.txt
reco2_diff.txt