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.

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

Jump to bottom

Fix reporting bug where ContextCounters have their per-context counts printed twice #497

Merged
merged 4 commits into from
May 3, 2024
Merged

Fix reporting bug where ContextCounters have their per-context counts printed twice #497

merged 4 commits into from
May 3, 2024

Conversation

bdutro
Copy link
Contributor

@bdutro bdutro commented Apr 30, 2024

I ran into a bug where the per-context counts from a ContextCounter would get printed twice in a report:

              my_counter = 882885
              my_counter_context0 = 221216
              my_counter_context1 = 220455
              my_counter_context2 = 220124
              my_counter_context3 = 221090
              my_counter.context0 = 221216
              my_counter.context1 = 220455
              my_counter.context2 = 220124
              my_counter.context3 = 221090

This PR fixes the report generator so that only the second set (with the . separator) is included in the report.

@bdutro bdutro requested a review from klingaard April 30, 2024 18:17
bdutro
bdutro commented Apr 30, 2024
View reviewed changes
sparta/src/Report.cpp Outdated Show resolved Hide resolved
klingaard
klingaard approved these changes May 2, 2024
View reviewed changes
Copy link
Member

@klingaard klingaard left a comment

Choose a reason for hiding this comment

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

Approval pending on the explicit construction...

@bdutro
Make all single-argument StatisticInstance constructors explicit
6944bdb 
Change StatAdder operator() to take const TreeNode* instead of TreeNode*
@bdutro bdutro merged commit 2f3dc4c into master May 3, 2024
8 checks passed
@bdutro bdutro deleted the dev/bdutro/fix-context-counter-reporting branch May 3, 2024 14:30
github-actions bot pushed a commit that referenced this pull request May 3, 2024
@bdutro
[ci skip] Fix reporting bug where ContextCounters have their per-cont…
71dab5b 
…ext counts printed twice (#497)

* Fix reporting bug where ContextCounters have their per-context counts
printed twice

* Update Doxygen comments for add() methods

* Second (simpler) attempt at fixing duplicate ContextCounters in reports

* Make all single-argument StatisticInstance constructors explicit
Change StatAdder operator() to take const TreeNode* instead of TreeNode* 2f3dc4c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@klingaard klingaard klingaard approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@bdutro @klingaard