-
Notifications
You must be signed in to change notification settings - Fork 15
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.
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
Changes from 6 commits
a00768e
1e57f4e
54c36d6
9a8c195
281f7ae
a276d60
600c56e
d81394a
a7a32c5
0a1c05a
d024f25
4875e26
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -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 | ||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Or you can tell us "no, thank you".
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||||||||||
|
|
||||||||||
|
|
||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,8 +12,8 @@ namespace caf | |
| { | ||
| public: | ||
| SRCRTSpacePointMatch(); | ||
| virtual ~SRCRTSpacePointMatch() {} | ||
|
|
||
| bool matched; // whether or not a match was made | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ... and could you add Doxygen format...
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 ( |
||
| SRCRTSpacePoint spacepoint; // the spacepoint | ||
| float score; // assessment of quality of matching (depends on alg configuration) | ||
| }; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 And blah blah Doxygen format blah. |
||
| }; | ||
| } // end namespace | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,8 +14,9 @@ namespace caf | |
| SRSBNDCRTTrackMatch(); | ||
| virtual ~SRSBNDCRTTrackMatch() {} | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you please remove the offending line above ( |
||
|
|
||
| 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) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Grumble Doxygen grumble ggro. |
||
| }; | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest the
booldata 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 fromclasses_def.xml).There was a problem hiding this comment.
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! 😅