Skip to content

Conversation

@henrylay97
Copy link
Member

@henrylay97 henrylay97 commented Feb 17, 2025

Description

This adds to the CRT calibration service with a list of bad channels. Like the pedestals and timing offsets these are filled from a text file. The service then denotes the channels (and their paired channel) with the correct labels using the enumeration provided in SBNSoftware/sbnobj#119. Only bad channels are specified - the default value is good (0).

The status is then used to skip over bad channels in the first stage of the CRT reconstruction. (It is also used in an Analyzer used for CRT calibrations which will soon be provided in a separate PR).

The PR will remain a draft until after this week's CRT meeting where a decision on the two channels labelled as quiet (3) will be made.

Checklist

  • Added at least 1 label from available labels.
  • Assigned at least 1 reviewer under Reviewers,
  • Assigned all contributers including yourself under Assignees
  • Linked any relevant issues under Developement
  • 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)

Requires the merging of SBNSoftware/sbnobj#119

Link(s) to docdb describing changes (optional)

https://sbn-docdb.fnal.gov/cgi-bin/sso/ShowDocument?docid=39940

@henrylay97 henrylay97 added crt Cosmic Ray Tagger data features for data processing labels Feb 17, 2025
@henrylay97 henrylay97 requested a review from kjplows February 17, 2025 10:56
@henrylay97 henrylay97 self-assigned this Feb 17, 2025
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Picky comment, but if the enum changes in sbnobj/SBND/CRT/CRTEnums.h this status could break - would it be clearer to replace e.g.

182    30    1

with

182    30    kDeadChannel

?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leaving a comment here to pick up on after Wednesday's CRT meeting.
If we decide to keep quiet channels this will need to change

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we decided to keep the quiet channels, I'm requesting a change here. Thanks Henry!

@henrylay97
Copy link
Member Author

Great suggestions from John. I have removed all implicit conversions so that the enum is used properly. Will still wait to make a decision on the quiet channels before "un-drafting".

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we decided to keep the quiet channels, I'm requesting a change here. Thanks Henry!

@henrylay97
Copy link
Member Author

No disagreements in the CRT channel after follow up from Wednesday's meeting. Have implemented the required change. Only need to check on kDeadChannel not kDeadNeighbourChannel as the latter is only ever assigned as a pair to the former.

@kjplows
Copy link
Contributor

kjplows commented Feb 24, 2025

Looks good, thanks Henry!

@henrylay97 henrylay97 marked this pull request as ready for review February 27, 2025 09:01
@kjplows kjplows moved this from Open pull requests to Testing in SBN software development Feb 28, 2025
@bear-is-asleep
Copy link
Contributor

trigger build LArSoft/lar*@LARSOFT_SUITE_v10_04_04 SBNSoftware/sbncode@v10_04_04

@FNALbuild
Copy link
Collaborator

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

@FNALbuild
Copy link
Collaborator

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

@FNALbuild
Copy link
Collaborator

❌ 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
Copy link
Collaborator

⚠️ CI build for SBND Warning at phase ci_tests SBND on slf7 for e26:prof - ignored warnings for build -- details available through the CI dashboard

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

parent CI build details are available through the CI dashboard

@bear-is-asleep
Copy link
Contributor

trigger build LArSoft/lar*@LARSOFT_SUITE_v10_04_03 SBNSoftware/sbnanaobj@v09_23_03 SBNSoftware/sbnobj@v10_00_04 SBNSoftware/sbncode@v10_04_03

@FNALbuild
Copy link
Collaborator

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

@FNALbuild
Copy link
Collaborator

❌ 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
Copy link
Collaborator

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

@FNALbuild
Copy link
Collaborator

⚠️ CI build for SBND Warning at phase ci_tests SBND on slf7 for e26:prof -- details available through the CI dashboard

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

parent CI build details are available through the CI dashboard

@bear-is-asleep
Copy link
Contributor

Approved

@kjplows
Copy link
Contributor

kjplows commented Mar 7, 2025

trigger build LArSoft/lar*@LARSOFT_SUITE_v10_04_05 SBNSoftware/sbnanaobj@v10_00_00 SBNSoftware/sbnobj@v10_00_05 SBNSoftware/sbncode@v10_04_04

@FNALbuild
Copy link
Collaborator

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

@FNALbuild
Copy link
Collaborator

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

@FNALbuild
Copy link
Collaborator

❌ 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
Copy link
Collaborator

⚠️ CI build for SBND Warning at phase ci_tests SBND on slf7 for e26:prof - ignored warnings for build -- details available through the CI dashboard

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

parent CI build details are available through the CI dashboard

@henrylay97
Copy link
Member Author

@RachelCoackley @bear-is-asleep do we understand these product differences? The true particle stuff in the CAF tests. They aren't related to this PR and they seem to be appearing in lots of PR tests?

@RachelCoackley
Copy link
Contributor

@henrylay97 @bear-is-asleep I'm not too sure what these CAF differences are, but as a guess are they related to this merged PR in sbnanaobj

@RachelCoackley
Copy link
Contributor

⚠️ CI build for SBND Warning at phase ci_tests SBND on slf7 for e26:prof -- details available through the CI dashboard

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

parent CI build details are available through the CI dashboard

I can't see these caf differences in this trigger of the CI where v09_23_03 of sbnanaobj was used (which was the version used when creating the current ref files). Looks like the PR I linked above was added between v09_23_03 and v10_00_00 of sbnanaobj: SBNSoftware/sbnanaobj@v09_23_03...v10_00_00. We can update ref files to reflect this change when a new version of sbndcode comes out (and then those differences shouldn't appear anymore 🤞)

@kjplows
Copy link
Contributor

kjplows commented Mar 10, 2025

Hi all, I've just checked the phase logs for nucosmics and can see the following:

4107: Checking record 0
4108:   rec.slc[0].reco.pfp[2].shw.producer differs: 9999999 vs 0
4109:   rec.slc[0].reco.pfp[2].shw.truth.p.end_process differs: 63 vs 1024
4110:   rec.slc[0].reco.pfp[2].shw.truth.p.start_process differs: 63 vs 1024
4111:   rec.slc[0].reco.pfp[2].trk.bestplane differs: -1 vs 1
4112:   rec.slc[0].reco.pfp[2].trk.truth.p.end_process differs: 63 vs 1024
4113:   rec.slc[0].reco.pfp[2].trk.truth.p.start_process differs: 63 vs 1024
4114:   rec.slc[1].reco.pfp[5].shw.producer differs: 9999999 vs 0
4115:   rec.slc[1].reco.pfp[5].shw.truth.p.end_process differs: 63 vs 1024
4116:   rec.slc[1].reco.pfp[5].shw.truth.p.start_process differs: 63 vs 1024
4117:   rec.slc[1].reco.pfp[5].trk.bestplane differs: -1 vs 0
4118:   rec.slc[1].reco.pfp[5].trk.truth.p.end_process differs: 63 vs 1024
4119:   rec.slc[1].reco.pfp[5].trk.truth.p.start_process differs: 63 vs 1024
...

I can comment on the lines with start_ and end_process - these come from sbnanaobj/#135 that @RachelCoackley correctly pointed out, as the kG4UNKNOWN variable was incremented to 1024.

@kjplows
Copy link
Contributor

kjplows commented Mar 10, 2025

As per the suggestion to update ref files - that sounds to me like a good idea, especially when dependencies to sbnanaobj, sbnobj, and potentially sbncode are updated. These first two repos will most likely lead to changes in commonly used enums that probably show up in the CI.

@RachelCoackley
Copy link
Contributor

rec.slc[0].reco.pfp[2].shw.producer differs: 9999999 vs 0 and rec.slc[0].reco.pfp[2].trk.bestplane differs: -1 vs 1 are related to sbnanaobj#136 where I set default values for these variables and updated the refs with these default values. This PR wasn't included in the trigger (hence the differences!!) 😊

@bear-is-asleep bear-is-asleep merged commit 06ca4b5 into develop Mar 10, 2025
3 of 4 checks passed
@github-project-automation github-project-automation bot moved this from Testing to Done in SBN software development Mar 10, 2025
@bear-is-asleep bear-is-asleep moved this from Draft to In tagged release in SBND March 2025 production Mar 12, 2025
@kjplows kjplows moved this from Done to 2025 PRs in SBN software development Jan 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

crt Cosmic Ray Tagger data features for data processing

Projects

Status: 2025 PRs
Status: In tagged release

Development

Successfully merging this pull request may close these issues.

6 participants