Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CRTPMT bug fix #764

Open
SFBayLaser opened this issue Oct 17, 2024 · 6 comments
Open

CRTPMT bug fix #764

SFBayLaser opened this issue Oct 17, 2024 · 6 comments
Assignees

Comments

@SFBayLaser
Copy link
Contributor

SFBayLaser commented Oct 17, 2024

A bug was found in the CRT/PMT matching which now has a fix.
This should be propogated back to the branch to be used for analysis.

@cerati
Copy link
Contributor

cerati commented Oct 22, 2024

we would need to back port #760.
@francescopoppi is this affecting only the CAF stage or also stage1?

@SFBayLaser
Copy link
Contributor Author

This is the problem/fix along the develop branch: SBNSoftware/sbncode#477
This provides an explanation of the issue.

@SFBayLaser
Copy link
Contributor Author

Follow up with Francesco on the question of whether we need to reprocess the data vs rerun CAF making:

The change from Anna is "good enough" with some caveat:
The CRT PMT matching stores a flashClassification and the relative time difference between flash and the CRTs.
In absence of issues, checking both flashClassification and the relative time difference is redunt.
But there was a bug (fixed in the producer). Anna's fix to CAFs make it so that the analyzers can use the delta-T in CAFs to evaluate their selection. This will solve the need to rerun the production. In this case the analyzers can ignore the flashClassification variable, which in some cases is wrong.

One thing we can do, to make it even more analyzers proof, is asking Anna to include in her PR the re-evaluation of the flashClassification.

so basically we can make it so that the flashClassification gets automatically filled correct and the analyzers are agnostic of any issues.

I can ask Anna to include that.

@SFBayLaser
Copy link
Contributor Author

We now have @francesco's bug fix for the producer module in the v09_89_01_01p03 release. But I understand from this morning's infrastructure meeting that we also need Anna's fix? Can either @francesco or @anna comment? If we need another PR can we get that this week based on the SBN2024A branch for the next release along this branch? Thanks!

@aheggest
Copy link
Contributor

hi @SFBayLaser, my fix only affects the CAFMaker. v09_89_01_01p03 could be fine for production to start with and we could rerun the CAFMaker on the stage1 files to pick up my updates to the CAF entries. I do have the fix ready in an sbncode PR based on develop and am currently making a sbncode PR based on SBN2024A.

@francescopoppi
Copy link
Contributor

@SFBayLaser #786 (#787 for develop) Should close this issue once for all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants