Skip to content

Extra SBND CRT Branches #186

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.

Sign up for GitHub

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

Merged
hgreenlee merged 12 commits into from
Mar 13, 2026
Merged

Extra SBND CRT Branches #186

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
a00768e
Add tagger and nhits fields to CRT space point representation
henrylay97 Feb 17, 2026
1e57f4e
Add tagger vector field to CRT track representation
henrylay97 Feb 17, 2026
54c36d6
Update classes_def.xml
henrylay97 Feb 17, 2026
9a8c195
Add matched flags for SBND TPC-CRT matching
henrylay97 Feb 18, 2026
281f7ae
Update the classes_def
henrylay97 Feb 18, 2026
a276d60
Apply good suggestions from Gianluca
henrylay97 Mar 13, 2026
600c56e
Add required header
henrylay97 Mar 13, 2026
d81394a
Re-order members
henrylay97 Mar 13, 2026
a7a32c5
Doxygen-ify the comments and add docDB link
henrylay97 Mar 13, 2026
0a1c05a
Re-order variables
henrylay97 Mar 13, 2026
d024f25
Remove unnecessary functions
henrylay97 Mar 13, 2026
4875e26
Update checksums, only keeping most recent from this development
henrylay97 Mar 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion sbnanaobj/StandardRecord/SRCRTSpacePoint.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ namespace caf
pe(std::numeric_limits<float>::signaling_NaN()),
time(std::numeric_limits<float>::signaling_NaN()),
time_err(std::numeric_limits<float>::signaling_NaN()),
complete(false)
complete(false),
Copy link
Copy Markdown
Member

@PetrilloAtWork PetrilloAtWork Mar 12, 2026

Choose a reason for hiding this comment

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

I suggest the bool data members to be moved at the end for memory layout reasons ([https://sbnsoftware.github.io/sbn/codingconv/CodingConventionsExplained.html#CF152]). Not really important (but it might change checksum, in which case the transient one should be removed from classes_def.xml).

Copy link
Copy Markdown
Member Author

@henrylay97 henrylay97 Mar 13, 2026

Choose a reason for hiding this comment

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

Even on my last day I am learning things from you Gianluca! 😅

nhits(std::numeric_limits<int>::lowest()),
tagger(std::numeric_limits<int>::lowest())
Comment thread
henrylay97 marked this conversation as resolved.
Outdated
{}
} // end namespace caf
////////////////////////////////////////////////////////////////////////
2 changes: 2 additions & 0 deletions sbnanaobj/StandardRecord/SRCRTSpacePoint.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ namespace caf
float time; // time [ns]
float time_err; // time_err [ns]
bool complete; // was cluster made from perpendicular & overlapping strips?
int nhits; // the number of strip hits contributing to the space point
int tagger; // the tagger the space point is on
Copy link
Copy Markdown
Member

@PetrilloAtWork PetrilloAtWork Mar 12, 2026

Choose a reason for hiding this comment

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

It's not your responsibility, but it would be a service to us if you could turn all these comments (not only the two new lines from your PR) into Doxygen comments, e.g.

Suggested change
int nhits; // the number of strip hits contributing to the space point
int tagger; // the tagger the space point is on
int nhits; ///< The number of strip hits contributing to the space point.
int tagger; ///< The tagger the space point is on.

Or you can tell us "no, thank you".

Copy link
Copy Markdown
Member Author

@henrylay97 henrylay97 Mar 13, 2026

Choose a reason for hiding this comment

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

No no it's a good shout. I don't have time to do a sweep of other classes I'm responsible for but I'll do these!

};
} // end namespace

Expand Down
1 change: 1 addition & 0 deletions sbnanaobj/StandardRecord/SRCRTSpacePointMatch.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
namespace caf
{
SRCRTSpacePointMatch::SRCRTSpacePointMatch():
matched(false),
score(std::numeric_limits<float>::signaling_NaN())
{}
}
Expand Down
1 change: 1 addition & 0 deletions sbnanaobj/StandardRecord/SRCRTSpacePointMatch.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ namespace caf
SRCRTSpacePointMatch();
virtual ~SRCRTSpacePointMatch() {}
Comment thread
henrylay97 marked this conversation as resolved.
Outdated

bool matched; // whether or not a match was made
Copy link
Copy Markdown
Member

@PetrilloAtWork PetrilloAtWork Mar 12, 2026

Choose a reason for hiding this comment

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

... and could you add Doxygen format...

Copy link
Copy Markdown
Member

@PetrilloAtWork PetrilloAtWork Mar 12, 2026

Choose a reason for hiding this comment

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

I suggest this data member to be moved at the end for C++ memory layout reasons ([CF-152]). If adopted, the initialiser in the constructor will also need to be moved accordingly.

SRCRTSpacePoint spacepoint; // the spacepoint
float score; // assessment of quality of matching (depends on alg configuration)
};
Expand Down
4 changes: 1 addition & 3 deletions sbnanaobj/StandardRecord/SRSBNDCRTTrack.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,7 @@ namespace caf
float time_err; // error in average time [ns]
float pe; // total PE;
float tof; // time from first space point to last [ns]

// TODO: Find way of adding taggers field
// std::set<SBNDCRTTagger_t> taggers; // which taggers were used to create the track
std::vector<int> taggers; // the taggers that were used to create the track
Copy link
Copy Markdown
Member

@PetrilloAtWork PetrilloAtWork Mar 12, 2026

Choose a reason for hiding this comment

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

If you have a reference at hand (DocDB or C++ class name) add it in the comment to help people interpret e.g. what tagger 1 means).

And blah blah Doxygen format blah.

};
} // end namespace

Expand Down
1 change: 1 addition & 0 deletions sbnanaobj/StandardRecord/SRSBNDCRTTrackMatch.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
namespace caf
{
SRSBNDCRTTrackMatch::SRSBNDCRTTrackMatch():
matched(false),
score(std::numeric_limits<float>::signaling_NaN())
{}
}
Expand Down
5 changes: 3 additions & 2 deletions sbnanaobj/StandardRecord/SRSBNDCRTTrackMatch.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,9 @@ namespace caf
SRSBNDCRTTrackMatch();
virtual ~SRSBNDCRTTrackMatch() {}
Copy link
Copy Markdown
Member

@PetrilloAtWork PetrilloAtWork Mar 12, 2026

Choose a reason for hiding this comment

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

Could you please remove the offending line above ([CF-151])? not your responsibility, etc.


SRSBNDCRTTrack track; // the track
float score; // assessment of quality of matching (depends on alg configuration)
bool matched; // whether or not a match was made
SRSBNDCRTTrack track; // the track
float score; // assessment of quality of matching (depends on alg configuration)
Copy link
Copy Markdown
Member

@PetrilloAtWork PetrilloAtWork Mar 12, 2026

Choose a reason for hiding this comment

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

Grumble Doxygen grumble ggro.

};
}

Expand Down
12 changes: 8 additions & 4 deletions sbnanaobj/StandardRecord/classes_def.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -416,19 +416,23 @@
</class>
<class name="std::vector<caf::SRTPCPMTBarycenterMatch>"/>

<class name="caf::SRCRTSpacePoint" ClassVersion="10">
<class name="caf::SRCRTSpacePoint" ClassVersion="11">
<version ClassVersion="11" checksum="2928297658"/>
<version ClassVersion="10" checksum="3912601582"/>
</class>

<class name="caf::SRCRTSpacePointMatch" ClassVersion="10">
<class name="caf::SRCRTSpacePointMatch" ClassVersion="11">
<version ClassVersion="11" checksum="414814289"/>
<version ClassVersion="10" checksum="382456729"/>
</class>

<class name="caf::SRSBNDCRTTrack" ClassVersion="10">
<class name="caf::SRSBNDCRTTrack" ClassVersion="11">
<version ClassVersion="11" checksum="1038334522"/>
<version ClassVersion="10" checksum="1967800431"/>
</class>

<class name="caf::SRSBNDCRTTrackMatch" ClassVersion="10">
<class name="caf::SRSBNDCRTTrackMatch" ClassVersion="11">
<version ClassVersion="11" checksum="3360963232"/>
<version ClassVersion="10" checksum="2486605052"/>
</class>

Expand Down