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

Adding offbeam gate counting in CAFs #79

Closed
wants to merge 6 commits into from

Conversation

jzettle
Copy link

@jzettle jzettle commented Sep 30, 2022

Added offbeam gate counting in CAFs adding a similar object to what is added for BNB and NuMI spill accounting. Looks at the number of gates for BNB and NuMI offbeam alone for offbeam data which event-by-event is computed as the difference in the number of gates for the previous event.

@jzettle jzettle requested a review from jzennamo September 30, 2022 21:48
@jzettle
Copy link
Author

jzettle commented Sep 30, 2022

This is a set of PRs that spans SBNSoftware/sbnana#89, SBNSoftware/sbncode#298, and SBNSoftware/sbnobj#67

@@ -31,6 +32,7 @@ namespace caf
unsigned int fno; ///< Index of file processed by CAFMaker
unsigned int ngenevt; ///< Number of events generated in input file associated with this record (before any filters)
float pot; ///< POT of the subrun associated with this record
float extcount;
Copy link

Choose a reason for hiding this comment

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

It looks like the number of "beam gates" is missing, which would be needed to normalize "extcount" to "pot".

Also, it looks like "extcount" is missing a comment describing it.

Comment on lines +12 to +13
isMajority(false),
isMinBias(false)

Choose a reason for hiding this comment

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

These should be commented what they mean or their name should be changed to something more generic.

Comment on lines +13 to +14
bool isMajority;
bool isMinBias;

Choose a reason for hiding this comment

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

These should be commented what they mean or their name should be changed to something more generic.

Copy link
Member

@PetrilloAtWork PetrilloAtWork left a comment

Choose a reason for hiding this comment

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

Fortunately the integration test caught the error somehow...
This commit is trying to redefine the checksum of caf::SRHeader version 17 which was previously there. This must not be, since version 17 was seen in a release and data may have been produced with it.
A new version 18 should be added instead:

<class name="caf::SRHeader" ClassVersion="18">
  <version ClassVersion="18" checksum="2118058550"/>
  <version ClassVersion="17" checksum="2390355049"/>

@miquelnebot miquelnebot added the help wanted Extra attention is needed label Feb 7, 2023
@miquelnebot miquelnebot marked this pull request as draft February 7, 2023 10:24
@jzettle jzettle closed this Jul 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants