Move crt space points to reco1 so Supera can consume them

[bear-is-asleep]

Description

Moves CRT space point making to reco1 so Supera can use them.

Adds ~0.003 s to reco1 stage processing time and increases the file size at reco1 by less than 0.1%.

Sequential tests were performed here.

Checklist

• Added at least 1 label from available labels.
• Assigned at least 1 reviewer under Reviewers,
• Assigned all contributers including yourself under Assignees
• [na] Linked any relevant issues under Developement
• [na] Does this PR affect CAF data format? If so, please assign a CAF maintainer (PetrilloAtWork or JosiePaton) as additional reviewer.
• Does this affect the standard workflow?

Relevant PR links (optional)

Supera PR #58 is dependent on this PR.

[henrylay97]

So I don't strongly disagree with this change as the CRT reco is pretty light both in resource usage and file size. I would like to hear @absolution1's thoughts though.

Is there a reason not to also do this for data?

[bear-is-asleep]

So I don't strongly disagree with this change as the CRT reco is pretty light both in resource usage and file size. I would like to hear @absolution1's thoughts though.

Is there a reason not to also do this for data?

No reason, I updated this now.

[absolution1]

Hi @bear-is-asleep

Thanks for the PR.

Could you do a quick check that the resource usage doesn't shift significantly for reco1
It would also be good to remove the fcl include that @henrylay97 . It sounds like its a relevant change.

Provided that we're only shifting light resources from reco1 to reco2 then I could see other definite benefits in this change (besides outside of supera consumption) e.g. opening up the possibilities of doing more studies using solely reco1

[bear-is-asleep]

Hi @bear-is-asleep

Thanks for the PR.

Could you do a quick check that the resource usage doesn't shift significantly for reco1 It would also be good to remove the fcl include that @henrylay97 . It sounds like its a relevant change.

Provided that we're only shifting light resources from reco1 to reco2 then I could see other definite benefits in this change (besides outside of supera consumption) e.g. opening up the possibilities of doing more studies using solely reco1

Yes the CI should test for resource usage differences, which I can carefully view and report in a comment.

[bear-is-asleep]

trigger build LArSoft/lar*@LARSOFT_SUITE_v10_04_06 SBNSoftware/sbncode@v10_04_06_01

[FNALbuild]

✔️ CI build for LArSoft Succeeded on slf7 for c14:prof -- details available through the CI dashboard

[FNALbuild]

❌ CI build for SBND Failed at phase build SBND on slf7 for c14:prof -- details available through the CI dashboard

🚨 For more details about the failed phase, check the build SBND phase logs

parent CI build details are available through the CI dashboard

[FNALbuild]

✔️ CI build for LArSoft Succeeded on slf7 for e26:prof -- details available through the CI dashboard

[FNALbuild]

❌ CI build for SBND Failed at phase ci_tests SBND on slf7 for e26:prof -- details available through the CI dashboard

🚨 For more details about the failed phase, check the ci_tests SBND phase logs

parent CI build details are available through the CI dashboard

[henrylay97]

Looks like the reference files might need updating? Not all of these changes result from this PR?

The errors in data production for reco2 are as a result of this - as we'd expect, modules looking for objects that are no longer produced by reco2. We should run the sequential test suite to check everything runs smoothly.

[bear-is-asleep]

trigger build LArSoft/lar*@LARSOFT_SUITE_v10_04_06 SBNSoftware/sbncode@v10_04_06_01

[bear-is-asleep]

Looks like the reference files might need updating? Not all of these changes result from this PR?

The errors in data production for reco2 are as a result of this - as we'd expect, modules looking for objects that are no longer produced by reco2. We should run the sequential test suite to check everything runs smoothly.

Updated refs!

[FNALbuild]

✔️ CI build for LArSoft Succeeded on slf7 for e26:prof -- details available through the CI dashboard

[FNALbuild]

✔️ CI build for LArSoft Succeeded on slf7 for c14:prof -- details available through the CI dashboard

[FNALbuild]

❌ CI build for SBND Failed at phase build SBND on slf7 for c14:prof -- details available through the CI dashboard

🚨 For more details about the failed phase, check the build SBND phase logs

parent CI build details are available through the CI dashboard

[FNALbuild]

❌ CI build for SBND Failed at phase ci_tests SBND on slf7 for e26:prof -- details available through the CI dashboard

🚨 For more details about the failed phase, check the ci_tests SBND phase logs

parent CI build details are available through the CI dashboard

[henrylay97]

Thanks @bear-is-asleep current CI results look sensible to me.

fcl_checks and reco1 tests show expected changes from this PR
reco2 doesn't run because it can't find CRT Clusters,SpacePoints or Tracks from previous stage - makes sense

The changes in detsim & caf are NOT resulting from this PR. These are currently known non-reproducible differences is that right?

I still think we should run the sequential tests on this PR @bear-is-asleep @RachelCoackley

[bear-is-asleep]

tests on this PR

Sorry I've been shifting so not really around in my mornings. Are there changes needed? I don't think we drop any CRT products at reco1, so they should be found at reco2. I tested this locally.

I think this is what you mean by needing sequential tests @henrylay97 , but I'm unsure how to trigger them. @RachelCoackley is no longer the CI manager so we'd have to ping Vito if this is what we need.

[bear-is-asleep]

Hi @bear-is-asleep

Thanks for the PR.

Could you do a quick check that the resource usage doesn't shift significantly for reco1 It would also be good to remove the fcl include that @henrylay97 . It sounds like its a relevant change.

Provided that we're only shifting light resources from reco1 to reco2 then I could see other definite benefits in this change (besides outside of supera consumption) e.g. opening up the possibilities of doing more studies using solely reco1

Hi @absolution1 , I did a few tests. The product size difference is < 0.1%, and the process time for these is around 0.001 s for each additional stage. I agree these fit in more with reco1 (making space points especially).

[henrylay97]

No @bear-is-asleep the CI only runs the stage in question. So reco2 expects the CRTReco to have been run in reco1 but that is not the case in the reference files.

We will need the sequential test suite run to check this. If Rachel is no longer doing CI that complicates things. I haven't got the time myself to look into this until at the earliest mid next week.

[bear-is-asleep]

Approved offline by @henrylay97 and @dombarker30

[henrylay97]

@absolution1 not Dom Barker