LLNL / variorum

Vendor-neutral library for exposing power and performance features across diverse architectures
https://variorum.readthedocs.io
MIT License
69 stars 26 forks source link

Support a choice between delta from the first call versus previous call in `variorum_get_energy_json` #575

Open tpatki opened 3 months ago

tpatki commented 3 months ago

Update the API to support nested calls in general, especially in Caliper-like tools. This might be useful for the Kokkos-update as well.

Merge #559 and #563 first, and then add a new flag to the API.

Current suggestion: variorum_get_energy_json(char** s) will be updated to variorum_get_energy_json(char** s, bool prev_delta). Setting prev_delta to true will return the accumulated energy since the previous call to the variorum_get_energy_json function from the application/tool's context. Setting this to false will return the accumulated energy since the first call to the variorum_get_energy_json.

I will work on an initial WIP PR as soon as I can, hoping to get this merged in by end of August. Happy to take any feedback and suggestions on this.

rountree commented 3 months ago

@tpatki How does prev_delta=false handle counter rollover? Are we guaranteed to have a thread that's sampling in the background often enough to detect that?

tpatki commented 3 months ago

Hi @rountree That's a great question, and it will vary by the underlying architecture. See details below.

(I am hoping these notes will also help @dbo understand the challenges at our end and why supporting this will take some time.)

Let me know if I answered that in enough detail, happy to have a meeting next week to discuss.

tpatki commented 3 months ago

@slabasan @rountree Checking in if you had more questions or feedback here. If not, I can take a stab at a PR so we can test this out in Caliper (and also add better support for our Kokkos users).

rountree commented 3 months ago

@tpatki

So yes, I'd prefer to have the general case be sampling occasionally unless the vendor documentation we have makes rollover a once-per-decade thing. But I'm not implementing this, so it's just a preference.

tpatki commented 3 months ago

Thanks @rountree.

Given that we have limited resources for Variorum at the moment, at least for the first cut at this, I am going to lean toward telling the user that we are passing along data that the vendor libraries are providing us (ESMI, RSMI, NVML, etc) and trusting that these vendor APIs take care of rollovers.

On some architectures (e.g. all GPUs), the low-level registers are not accessible at all, and we have no choice but to trust that APIs such as nvmlDeviceGetTotalEnergyConsumption will do the right thing -- which I believe they do. GPU APIs report energy values based on when the driver was last loaded -- see here, so they would have to take care of any rollovers (although we've not explicitly tested this).

Intel CPUs are the only exception to this situation, where we read directly from the MSR_PKG_ENERGY_STATUS or MSR_DRAM_PKG_STATUS in our code. These have 32-bits of energy data and are updated every millisecond, resulting in a wraparound every few minutes.

Looking at our port, I realized that we are already taking care of wraparounds for these registers in the Intel port when we calculate deltas, as we need to do this for reporting power on these systems too. Take a look here.

My understanding is that we will be reporting the correct values for energy with the current Intel port if we chose to do deltas (no sampling will be needed if I am understanding the code correctly, but I haven't refreshed my memory on this port enough yet). I will have to test this explicitly when I start working on this PR. I believe @slabasan has tested these wraparounds before, she may be able to comment as well.

TLDR: Let's try to get a first cut at this while trusting the vendor APIs (and our Intel port). Let's document this well and explain to the users this decision. And let's leave an issue open to test for rollovers on each architecture, so we can fix these if we run into them or if any users run into them.

rountree commented 3 months ago

@tpatki Sounds good.