docs: restructure three explanation pages - #1372
Open
davidberenstein1957 wants to merge 3 commits into
Open
Conversation
Move the 165-line raw machine dumps in rapl.md into a clearly marked appendix so the explanation reads end to end. Retitle power-estimation.md to "From Energy Counters to Power" — the old title promised the TDP estimation model and delivered counter-to-power derivation. File path unchanged so external URLs keep working. Relabel model-comparisons.md: the numbers come from Azure tooling, not CodeCarbon. Provenance moved to a warning at the top and the nav entry moved out of Explanation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The three worked examples asserted CodeCarbon "uses psys only", which contradicted the Key Takeaways section on the same page. Verified against core/cpu.py: rapl_prefer_psys defaults to False (:453) and psys is used only when explicitly enabled (:694), with the else path logging "psys domain detected but not used" (:727). Key Takeaways was right. Each example now states the default package-domain behaviour and marks the psys arithmetic as requiring rapl_prefer_psys=True. Also reworded the open "we still have to investigate" note on DRAM as a stated limitation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The Threadripper walkthrough presents a summed per-die figure as correct. Whether the two package-X-die-Y domains are independent or mirror the same counter is unresolved, so the page now links issue #1379 instead of vouching for the result. Observed numbers left untouched as evidence. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Structural cleanup of three
explanation/pages. No technical claims were changed — only ordering, titles and labelling.explanation/rapl.mdThe 165 lines of annotated terminal dumps from three real machines sat in the middle of the page, between the domain-selection strategy and the key takeaways. Moved verbatim to the end as
## Appendix: raw measurements from three machines, with a note saying the reader does not need it to follow the explanation. Nothing deleted — the dumps are evidence.explanation/power-estimation.mdThe title promised the TDP/power estimation model; the content is about deriving power from energy counters (
Power.from_energies_and_delay, per-interval averaging, counter wrap, smoothing). Retitled to From Energy Counters to Power, with a short info box at the top pointing readers looking for the TDP/constant/cpu_load path tomethodology.md.The file was not renamed — external URLs and the existing
introduction/power_estimation.mdredirect keep working. Nav entry and the two inbound links (how-to/troubleshooting.md,how-to/enable-rapl.md) updated to the new title.explanation/model-comparisons.md— relabelled, not deletedKept rather than deleted: the table and charts are genuinely useful orders-of-magnitude context and are cited to published research, and deleting the page would break the
introduction/model_examples.mdredirect and any external links.But the provenance was buried at line 36 ("emissions equivalent are estimated using Microsoft Azure cloud tools"). Now it is a warning admonition in the first screen, the title reads Model Comparisons (Third-Party Data), and the nav entry moved out of the Explanation section (into Reference) so it no longer reads as CodeCarbon output. The file path is unchanged for the redirect's sake.
Checks
uv run task docspasses: build clean, link check clean (4628 attributes).mkdocs.ymlnav and may conflict with the concurrent methodology PR (which addsreferences.md/equivalences.mdto the same nav block). Trivial to resolve — both changes are additive lines in the Explanation list.🤖 Generated with Claude Code
Beyond restructuring,
rapl.mdcontradicted itself about the default RAPL domain, and moving the dumps into an appendix made the contradiction more visible. Verified against the source and fixed:codecarbon/core/cpu.py:453—rapl_prefer_psys=Falseis the constructor default.codecarbon/core/cpu.py:694— the psys branch isif self.rapl_prefer_psys and psys_domains:, so psys is used only when explicitly enabled.codecarbon/core/cpu.py:698-731— the default path selects package domains and logs "psys domain detected but not used (rapl_prefer_psys=False)".emissions_tracker.py:590— the tracker-level default is alsoFalse.So the Key Takeaways section was correct and the three worked examples were stale. Each "CodeCarbon behavior: uses psys only" assertion now states the default package-domain behaviour and marks the psys arithmetic as requiring
rapl_prefer_psys=True; a warning at the top of the appendix scopes the two Intel walkthroughs to that non-default setting. The raw numbers in the code blocks are untouched — each block already lists the package figure alongside psys, so the default reading is available from the same evidence.Also removed a "Legacy behavior (before v2.x): would have measured only package-0, missing ~14W" note, which asserted the exact inversion of current behaviour.
Separately, the "Actual DRAM chips, we still have to investigate on this point" line is reworded as a stated limitation (
dramreports memory-controller power; excluded by default viarapl_include_dram=False) rather than an open question in shipped docs.uv run task docsre-run after these edits: build clean, link check clean.Open correctness question linked (third commit)
The Threadripper walkthrough (
package-0-die-0: 68 W | package-0-die-1: 68 W | CodeCarbon: 137 W) presented a summed per-die figure as the intended result. The page no longer blesses that outcome: both the walkthrough and Key Takeaways item 7 now link issue #1379, stating neutrally that the Linux path sums every domain whose name containspackage, and that whether per-die domains are independent or mirror the same counter is unresolved.Nothing is asserted to be a bug, and the observed numbers are untouched — they are the evidence behind the issue. Key Takeaways item 7 still describes what the code does; only the claim that the outcome is correct is withdrawn.