docs: sync GPU power monitoring design doc with collector logic

[iacker]

What

Syncs the GPU power monitoring design doc (docs/developer/design/architecture/gpu-power-monitoring.md) with the current implementation in internal/device/gpu/nvidia/collector.go. Two sections had drifted from the code.

Why

I noticed this while reading the GPU collector, in the context of #2430 and #2497. The doc describes behaviour that no longer matches the code.

Idle Power Detection

The doc showed minObservedPower updated on every reading, and idle power resolved as a plain minimum lookup. getDevicePowerStatsLocked now does three things differently.

• It only updates the baseline when the GPU is truly idle. A Kepler start under load no longer sets a false baseline.
• It resolves idle power by precedence: configured SetIdlePower(), then observed idle, then a conservative zero.
• It clamps active power at zero, so negative active power is never reported.

MIG Mode

The doc still said per-instance attribution was not implemented, and showed the partitioned case skipped with continue. The collector attributes MIG power per instance via attributePartitioned, based on DCGM activity. attributePartitionedFallback distributes equally when DCGM is unavailable.

Change

Rewrites both sections to match the real control flow. Documentation only, no code changes.

[sunya-ch]

Is this something planned to introduce in the upcoming PR?
I have found that it is not duplicated from the current implementation:

kepler/internal/device/gpu/nvidia/collector.go

Lines 234 to 250 in 96430b4

	// Check if the GPU is truly idle (no compute processes running)
	procs, err := dev.GetComputeRunningProcesses()
	if err != nil {
	// Non-fatal: log and skip idle detection for this reading
	c.logger.Debug("GetComputeRunningProcesses failed, skipping idle detection",
	"device", deviceIndex, "error", err)
	} else if len(procs) == 0 {
	// GPU is truly idle — update minimum observed power
	if min, exists := c.minObservedPower[uuid]; !exists || totalPower < min {
	c.minObservedPower[uuid] = totalPower
	c.logger.Debug("updated idle power baseline",
	"device", deviceIndex, "uuid", uuid, "idlePower", totalPower)
	}
	c.idleObserved[uuid] = true
	}
	// Determine idle power:

[sunya-ch]

Thank you for the PR!

I have confirmed the following sync pointers.

kepler/internal/device/gpu/nvidia/collector.go

Line 319 in 96430b4

c.logger.Debug("MIG attribution failed",

kepler/internal/device/gpu/nvidia/collector.go

Line 254 in 96430b4

var idlePower float64

However, there is one point left where I cannot find the match source.

[iacker]

Thanks for the review @sunya-ch. On the MIG section, per-instance attribution is in the current implementation, it just sits further down than L234-250, which is the idle-detection path. The MIG logic the doc describes is in attributePartitioned:

• attributePartitioned
• GetMIGInstanceActivity query

So it is describing what is there today, not something planned.

[sunya-ch]

@iacker Actually, I want to point to the section Idle Power Detection.

I cannot find the source of the following code snippet that you place there. It seems to be a similar thing that L234-L250 does for checking if the GPU is truly idle but the code is different.

// Only update the observed baseline when the GPU is truly idle
// (no compute processes running).
procs, err := dev.GetComputeRunningProcesses()
if err == nil && len(procs) == 0 {
    if min, exists := c.minObservedPower[uuid]; !exists || totalPower < min {
        c.minObservedPower[uuid] = totalPower
    }
    c.idleObserved[uuid] = true
}

[iacker]

Good catch, you are right. That snippet was a paraphrase and had drifted from the real code. It collapsed the error path into err == nil && len(procs) == 0, which hides the non-fatal GetComputeRunningProcesses() failure branch that getDevicePowerStatsLocked actually has.

Fixed in e4bbbad. Both Go blocks in Idle Power Detection now match collector.go verbatim. The idle check keeps the real if err != nil { log & skip } else if len(procs) == 0 { ... } shape, so the doc no longer implies errors are treated as "not idle". The precedence switch is copied as-is, without the editorial comments that were not in the source.

[sunya-ch]

@iacker I rebased the branch but it turns me to co-author.
Could you please rebase the branch locally and force push here instead?

[iacker]

Both points addressed.

Snippet source: replaced the paraphrase with the verbatim block from getDevicePowerStatsLocked, the idle check plus the idlePower precedence switch and the activePower < 0 clamp. Added a [collector-idle] permalink to those lines so the doc stays traceable to the source. It matches L234-L247 exactly now.

Rebase: done locally, no co-author trailer left, DCO green.