Feature/afilkins commissioning trees

[flaviacicala]

This PR includes:

• a new CRT reconstruction using sbnd::crt::CRTTrack, instead of the default sbn::crt::crtTrack. The latter creates a track out of every possible pair of two crt hits, while the new CRT reconstruction, developed by @henrylay97, has more reasonable criteria for forming tracks which consists of looking at the timing, and includes the possibility of a track with 2 or 3 points along it which accommodates the case of crossing both of the top CRTs and one other wall.
• HitDumper module now uses these new SBND tracks instead of the standard SBN ones. This includes switching over from using sbn::crt::CRTHit to using sbnd::crt::CRTSpacePoint. This was implemented by Amy Filkins.

[fjnicolas]

Hi @flaviacicala, thanks for the PR.

• What's the timeline for this PR to get in? i.e. do you need this for the production release?
• Also, can you rebase this PR? develop has diverged too much from v09_72_01, so the integration can be really difficult... Thanks!

[flaviacicala]

Hi Fran, when is the next production release? Rather than needing this in for a specific time, the commissioning team would like this to become the default process to make CRT tracks to fix some known artificial features seen on our side. What is a reasonable timeline to achieve this from your side?

I agree that a rebase is needed. I'll do that now.

[henrylay97]

Hi @flaviacicala really glad this is going in, thank you! Any luck on a rebase / develop merge? Happy to review once that's done and is easier to parse :D

[fjnicolas]

thanks @flaviacicala for the last commit! @henrylay97 @marcodeltutto this is ready for review

[fjnicolas]

@henrylay97 @flaviacicala where do things stand for the two unresolved comments? Do you prefer more time to implement it or merging this in as it is right now? Thanks!

[flaviacicala]

@henrylay97 @flaviacicala where do things stand for the two unresolved comments? Do you prefer more time to implement it or merging this in as it is right now? Thanks!

I would merge this now to be sure that it actually goes in. We can make a new PR when the things we want to implement are done. What do you say @henrylay97 ?

[fjnicolas]

Sorry I've been really snowed with ✍️

@fjnicolas how imminent is the next release?

I actuallly have v09_87_00 ready to go but a bug was foud in the ROOT version used by that tag, so I'll probably skip this one... So, I don't think before next Monday (LArSoft published a patch fix yesterday, and it now needs to be propagated first to sbncode and then to SBND...).

If it isn't super imminent then let me see if I can find time to implement something for those two points in the next couple of days, both are pretty straightforward and would be useful to analysers imo. If I haven't found time when you're ready to cut a release then this can go in and I'll prepare a separate branch!

That sounds good! I'll make a last minute merge then.

[fjnicolas]

trigger build LArSoft/lar*@LARSOFT_SUITE_v09_87_00

[FNALbuild]

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

[FNALbuild]

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

[FNALbuild]

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

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

parent CI build details are available through the CI dashboard

[FNALbuild]

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

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

parent CI build details are available through the CI dashboard

[henrylay97]

Hi folks @flaviacicala @fjnicolas

Took a brief look over the weekend at filling the extra fields as promised. This branch, however, doesn't compile, as is clear from the CI tests too. I was able to quickly hack together a version that does compile & then I ran into fcl issues with the workflow, if you run the commissioning fcls you seem to be missing products that are needed when hit dumper gets run in reco1.

This concerns me a little that this branch is clearly not what has been being run in the offline. Relatively easy to get it to compile but could do with some input on what the workflow plans are from experts?

[fjnicolas]

Hi folks @flaviacicala @fjnicolas

Took a brief look over the weekend at filling the extra fields as promised. This branch, however, doesn't compile, as is clear from the CI tests too. I was able to quickly hack together a version that does compile & then I ran into fcl issues with the workflow, if you run the commissioning fcls you seem to be missing products that are needed when hit dumper gets run in reco1.

This concerns me a little that this branch is clearly not what has been being run in the offline. Relatively easy to get it to compile but could do with some input on what the workflow plans are from experts?

Hi @henrylay97 thanks for having a look! Probably all the CRT related hit producers need to be defined in run_hitdumper.fcl? In a similar wat yo fasthit ot ophitpmt... @flaviacicala can you confirm what were you running offline?

[henrylay97]

@fjnicolas absolutely, I just want to be careful about what I get myself into here. I'm happy to add a couple of features to something that is ready to go & validated. I don't want to spend time fixing a branch that isn't what the original team were using anyway :sweat_smile:

I'll let Flavia comment and we can go from there :smile:

[bear-is-asleep]

@henrylay97 @flaviacicala Hi both! What's the status of this? It would be nice to have this in since we will be getting that CRT data soon. It'll be useful for PMT+CRT channel mapping verification. Thanks!

[henrylay97]

Hi Bear! There is work going on offline to reproduce the intention of this branch. We'll get something to you as soon as we can! Beth McCusker is the main developer with help from me and Flavia.

[bear-is-asleep]

Hi Bear! There is work going on offline to reproduce the intention of this branch. We'll get something to you as soon as we can! Beth McCusker is the main developer with help from me and Flavia.

Thanks! Just checking in since I recently took over release manager duties.

[bear-is-asleep]

Closing upon the @henrylay97 request. To be superseded by PR 475