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

reset buffer stats in HostScoring #329

Merged
merged 2 commits into from
Jan 7, 2025

Conversation

SeverinDiederichs
Copy link
Collaborator

This PR fixes a critical bug in the HostScoring implementation:

The 3 variables required for the next hit slot

  hostScoring_dev->fBufferStart        = 0;
  *(hostScoring_dev->fNextFreeHit_dev) = 0;
  *(hostScoring_dev->fUsedSlots_dev)   = 0;

were not reset between events. Although they were ever increased via unsigned int next = hostScoring_dev->fNextFreeHit_dev->fetch_add(1); (and the fBufferStart was set to be the fNextHitFree), it worked due to the circular nature of the buffer return next % hostScoring_dev->fBufferCapacity;. However, as soon as the maximum int was exceeded, the simulation would simply stall and not do anything else anymore. This PR fixes that by resetting the three variables between events using a new function ResetBufferStatsGPU.

Note that there are a huge amount of changes due to the missing clang formatting in the file. The only other change was to reduce some input parameters for e.g., CopyHitsToHost and just use the hostscoring itself to get the stats.

@SeverinDiederichs SeverinDiederichs added the bug Type: Something isn't working label Jan 6, 2025
@phsft-bot
Copy link

Can one of the admins verify this patch?

Copy link
Contributor

@agheata agheata left a comment

Choose a reason for hiding this comment

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

Good for me, except resetting the counters in a new kernel launch, see comment inline. I wonder why the CI doesn't catch missing clang-format, we need to investigate.

include/AdePT/core/HostScoringImpl.cuh Show resolved Hide resolved
@agheata agheata merged commit 46aab71 into apt-sim:master Jan 7, 2025
2 checks passed
@SeverinDiederichs SeverinDiederichs deleted the reset_buffer_stats branch January 7, 2025 09:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Type: Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants