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

Feature/power #1479

Merged
merged 11 commits into from
Jul 9, 2024
Merged

Feature/power #1479

merged 11 commits into from
Jul 9, 2024

Conversation

maddyscientist
Copy link
Member

This PR is the initial step towards more power awareness in QUDA, as well as adding OMP threading for host kernels

  • Adds power, temperature and clock monitoring
    • Monitoring is enabled with QUDA_ENABLE_MONITOR=1 (default is off)
    • Monitoring is performed on a spawned thread, maintaining the history in a linked list
    • Default monitoring period is QUDA_ENABLE_MONITOR_PERIOD=1
    • Monitor info, together with derived energy usage is dumped to a monitor_*****.tsv file, where the **** encodes the rank id, and the date_time of the dump. All ranks have identical times by construction.
  • Add OpenMP threading support for all CPU kernels
    • Most kernels seem to get reasonable scaling
    • QUDA_OPENMP CMake parameter is no longer marked as advanced
  • Fixes compiler warning introduced in staggered contraction code #1416
  • Fixes bug introduced in staggered contraction code #1416
  • Fixed an issue with endQuda if memory leaks were detected when running multi-GPU: printfQuda would fail since comm_rank() would be called after the comms have been torn down

…e elsewhere. Move assertAllMemFree and device::destroy before we bring down the comms (fixes a potential issue where we call a QUDA i/o function in these functions but the comms is down
…toring period (in microseconds) is set by QUDA_ENABLE_MONITOR_PERIOD (Default = 1000 microseconds = 1 millisecond
@maddyscientist maddyscientist requested review from a team as code owners July 9, 2024 05:09
lib/monitor.cpp Outdated Show resolved Hide resolved
Copy link
Member

@bjoo bjoo left a comment

Choose a reason for hiding this comment

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

After a quick scan this looks fine to me, I had only a few OpenMP comments I left as single comments regarding the use of collapse() I saw you use it in some cases and not in others, but I was just wondering if it could be used in one or two more. The only thing I worry about is dropping a functor ( Arg::apply ) or some such into a reduction clause. I wonder if it ought to be a custom reduction (like done for Multi-Reductions).
This could be just me not being up to spec with my OpenMP though. Modulo these I approve.

@maddyscientist maddyscientist merged commit d199bd3 into develop Jul 9, 2024
13 of 14 checks passed
@maddyscientist maddyscientist deleted the feature/power branch July 9, 2024 22:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants