Skip to content

Revised the Implementation examples to comply with iso - #142

Open
seanmcilroy29 wants to merge 9 commits into
devfrom
Implementation-examples
Open

Revised the Implementation examples to comply with iso#142
seanmcilroy29 wants to merge 9 commits into
devfrom
Implementation-examples

Conversation

@seanmcilroy29

Copy link
Copy Markdown
Collaborator

Revised the Implementation examples to comply with iso

seanmcilroy29 and others added 8 commits March 26, 2025 10:05
Signed-off-by: Sean Mcilroy <smcilroy@linuxfoundation.org>
Signed-off-by: Sean Mcilroy <smcilroy@linuxfoundation.org>
Signed-off-by: gosia-f <gosia@greensoftware.foundation>
Signed-off-by: gosia-f <gosia@greensoftware.foundation>
Responded to comments to add a call out to include pre, mid and post training. Also added in about including test runs.


Signed-off-by: j-turnbull <jonturnbull@google.com>
Revised the Implementation examples to comply with iso

Signed-off-by: Sean Mcilroy <smcilroy@linuxfoundation.org>
@seanmcilroy29 seanmcilroy29 changed the title Implementation examples Revised the Implementation examples to comply with iso Jul 14, 2026
@claude

claude Bot commented Jul 14, 2026

Copy link
Copy Markdown

Review: Implementation examples (PR #142)

Thanks for aligning §9 with ISO-style formatting. Since this is a spec repo (not code), I focused feedback on numerical correctness, Markdown rendering, and scope. Per CLAUDE.md I checked FAQ.md — it does not reference these specific example figures or the section title, so no FAQ update is required for this change. Overall the substance is a net improvement; a few issues below.

What this fixes (nice catches)

This PR corrects several pre-existing numerical inconsistencies, and the new figures now reconcile internally:

  • LLM Consumer 130 g CO₂e/million tokens is now consistent with 130 kg CO₂e/billion tokens (the old 0.13 g/million was off by 1000x).
  • CV Provider 30,000 kg CO₂e/billion parameters now matches the calculation 75,000 / 2.5 = 30,000 (the old text had both 3000 and 30 in different places).

Rendering bug — EXAMPLE lines glued to the numbered list

In §9.1.1 and §9.1.2, the EXAMPLE ... line immediately follows list item 5. with no blank line:

5. Calculate Consumer SCI: `SCI = C / R`.
EXAMPLE   Total Consumer operational carbon of 5,000 kg CO2e/week ...

In Markdown this is a lazy continuation, so EXAMPLE ... renders as part of list item 5 rather than as its own paragraph. Add a blank line before each EXAMPLE. (§9.2.1/§9.2.2 are fine — they already have a separator.)

Trailing whitespace on blank lines

The reformat converted every blank separator line to a line containing a single space. This is pervasive across §9 and will trip Markdown linters / show up as diff noise. Please strip trailing whitespace so blank lines are truly empty.

Missing blank line before a heading

- Provider SCI: 4 g CO₂e/10¹⁵ FLOPs is immediately followed by ### 9.2 Computer vision model example with no blank line, unlike every other heading in the section. Add a blank line for consistency and safe rendering.

Scope: unrelated file added

.github/ISSUE_TEMPLATE/agenda.md (a meeting agenda/minutes template) is unrelated to "Implementation examples". It also carries an unfilled 2025.mm.dd placeholder and three duplicated "Add Topic" sections. Looks accidentally included — suggest pulling it into its own PR so this one stays focused.

Minor / optional

  • §9.1.2 boundary: adding Inception makes the LLM example consistent with the CV example (good), but the Calculation Method bullets below do not mention any Inception-stage emissions. Consider a one-line bullet so boundary and method agree.
  • README.md: the change drops the hyperlink on Navveen's name (plain text @navveenb instead of a link to the GitHub profile). Fine to add Henry, but you could keep both as links for consistency.

Nothing blocking beyond the EXAMPLE blank-line rendering fix. Happy to re-review once addressed.

Signed-off-by: Sean Mcilroy <smcilroy@linuxfoundation.org>
@claude

claude Bot commented Jul 31, 2026

Copy link
Copy Markdown

Review: PR #142 — Revised the Implementation examples to comply with ISO

Thanks for this. I checked the changed sections against FAQ.md, §5–§8 of SPEC.md, and the existing issue templates, per CLAUDE.md. The arithmetic is the strongest part of this PR — it silently fixes two real errors. Most of my comments are about scope and markdown rendering rather than content.


✅ Verified correct (and two bugs fixed)

I recomputed all four worked examples:

Example Calculation Result Status
9.1.1 Consumer (5,000 + 1,500) kg ÷ 50e9 tokens 130 kg/billion tokens
9.1.2 Provider (180,000 + 20,000) kg ÷ 5×10²² FLOPs 4×10⁻¹⁸ kg/FLOP = 4 g/10¹⁵ FLOPs
9.2.1 Consumer 3,200 kg ÷ 40e6 inferences 0.08 g/inference
9.2.2 Provider 75,000 kg ÷ 2.5e9 params 30,000 kg/billion params

Two pre-existing errors are corrected here, and they are worth calling out in the PR description so reviewers on the weekly call notice them:

  • §9.1.3 previously reported 0.13 g CO₂e/million tokens; the correct conversion of 130 kg/billion tokens is 130 g CO₂e/million tokens — a 1000× error, now fixed.
  • §9.2.2/9.2.3 previously disagreed with each other (3000 vs 30 kg/billion parameters) and both were wrong; 30,000 kg is correct and consistent in both places now. As a sanity check, 30,000 kg = 30 t CO₂e/billion parameters, which sits right inside the FAQ.md benchmark table (GPT-2 ~33.3 t, GPT-4 ~20 t — FAQ.md:161-165). The old value was three orders of magnitude below that table.

Also good: §9.1.2 adding Inception to the Provider boundary resolves a genuine mismatch with SPEC.md:132-136 (§6.2) and FAQ.md:92 / FAQ.md:302, and makes 9.1.2 consistent with 9.2.2. Per the CLAUDE.md rule, I checked FAQ.md for contradictions — nothing in it is invalidated by this PR, so no companion FAQ change is needed.


🔴 Rendering regression (unrelated to §9)

SPEC.md:228-231 — a blank line was inserted after > **Explanation**: inside a blockquote:

> **Explanation**:

> - **Gross values** refer to total quantities...

A blank line terminates a blockquote in CommonMark, so this splits one blockquote into two adjacent ones with a visible gap. Either revert the line, or use a bare > if vertical spacing was the intent:

> **Explanation**:
>
> - **Gross values** refer to total quantities...

This is in §8.2.2, outside the PR's stated scope — probably an accidental edit.


🟠 Scope: two unrelated files in an "Implementation examples" PR

.github/ISSUE_TEMPLATE/agenda.md (+51) and README.md (+2/-1) have nothing to do with ISO-aligning §9. Given the repo's process (issue → PR → weekly call review), bundling governance/meeting changes into a spec PR makes the call review harder and couples unrelated approvals. I'd suggest splitting these into their own PRs.

On agenda.md specifically, if it does stay:

  1. It's a near-duplicate of the existing agenda_minutes_template.md. The diff between the two is small — mostly *** separators replacing blank lines, plus dropped Previous Meeting Action Items / Weekly Reminders / AOB sections. Two nearly identical agenda templates in the picker will confuse whoever opens the next meeting issue. Is the intent to replace the existing one?
  2. No YAML frontmatter. motion-to-approve.md in the same directory has name: / about: / title: / labels: / assignees:. Without it this template appears in the chooser as the bare filename with no description and applies no labels. (The other .md templates share this gap, so it's arguably repo-wide — but new files shouldn't add to it.)
  3. Hardcoded year: ## 2025.mm.dd Agenda/Minutes — it's 2026. Use YYYY.mm.dd.
  4. Pre-checked boxes in a template: - [x] DD MMM and - [x] Motion to adjourn should be - [ ] so they're actionable when the issue is opened.
  5. Empty link: [previous Meeting Minutes]() renders as a dead link.
  6. Cross-repo link: the agenda-submissions line points at Green-Software-Foundation/standards-wg/issues, not this repo. Intentional?
  7. Name inconsistencies with the README in this same PR: Henry Richards here vs Henry Richardson in README.md; Naveen Balani here vs Navveen Balani in README.md. Worth getting both right in one pass.

On README.md:17 — replacing [Navveen Balani (Accenture)](https://github.com/navveenb) with plain text plus a bare @navveenb loses the hyperlink: GitHub does not autolink @mentions in repository markdown files (only in issues, PRs, and comments). Suggest keeping the link form for both:

* [Navveen Balani (Accenture)](https://github.com/navveenb)
* [Henry Richardson (WattTime)](https://github.com/Henry-WattTime)

🟡 ISO alignment is partial

The EXAMPLE convention and sentence-case headings are applied to §9 only, which leaves the document internally inconsistent:

  • SPEC.md:239 still uses the old style — **Example (Multi-Metric Reporting):** — immediately above the newly converted §9.
  • Headings: §9 is now sentence case; §1–§8 (## 2. Scope, ## 5. AI Lifecycle Stages, ### 8.1 Consumer Functional Units, …) remain Title Case.
  • Inline labels inside §9 kept Title Case: **Functional Unit**, **Calculation Method**, **Boundary**. Also SPEC.md:254 — "For a typical Large Language Model service" mid-sentence.
  • SPEC.md:166intermediate & test runs. ISO/IEC Directives Part 2 drafting rules prefer a spelled-out "and" over & in body text.

If §§1–8 are being handled by the sibling PRs (#137#141), a note in the description saying so would help; otherwise merging this alone leaves the spec in a visibly mixed state.

EXAMPLE + three spaces doesn't survive markdown rendering. ISO uses the wide gap for visual separation, but consecutive spaces collapse to one in rendered HTML, so the published doc shows EXAMPLE Total Consumer operational carbon of… as an ordinary paragraph. Consider **EXAMPLE** or a blockquote so the convention is actually visible in the rendered output, which is how this spec is consumed.

Relatedly — collapsing the bullet breakdowns into single run-on sentences costs real scannability:

EXAMPLE Total Provider operational emissions of 180,000 kg CO₂e plus total Provider embodied emissions of 20,000 kg CO₂e, divided by 5 × 10²² FLOPs used, gives a Provider SCI of 200,000 kg CO₂e ÷ (5 × 10²² FLOPs) = 4 × 10⁻¹⁸ kg CO₂e/FLOP = 4 g CO₂e/10¹⁵ FLOPs.

ISO/IEC Directives Part 2 permit lists inside examples, so the inputs could stay as bullets with only the label changing. Worth a decision on the call — this one is a judgement call, not a rule.


🟡 Content consistency

SPEC.md:280-292 — §9.1.2's boundary now correctly reads Inception, Design and Development, Deployment, Retirement, but the Calculation Method bullets beneath it still cover only data collection, training, optimization/testing, and system integration. As written, the example under-counts the boundary it just declared. Add bullets for the Inception, Deployment, and Retirement stages (e.g. project scoping and planning systems per §6.2, deployment infrastructure, decommissioning when material per §7.5).

Unit presentation differs between the two examples. §9.1.3 reports 130 g CO₂e/million tokens while its own EXAMPLE derived 130 kg CO₂e/billion tokens — same value, but the reader has to do the conversion. §9.2.3 keeps kg/billion parameters, matching its example. Pick one convention, or show the conversion explicitly in the EXAMPLE line.

Terminology (pre-existing, low priority): the boundary lists say Retirement, but §5.5 names the stage End of Life (SPEC.md:109). Since this PR normalizes stage enumerations it may be in scope — or defer to #140. Similarly, §7.5 says Retirement is "Consumer and Provider" while §6.1 scopes the Consumer boundary to Operation and Monitoring only; the Consumer examples here follow §6.1, so no change is needed in this PR, but that tension is worth its own issue.

Asymmetry: §9.1 subsections have a Calculation Method numbered list; §9.2 subsections have only Functional Unit / Boundary / EXAMPLE. Pre-existing, but this PR is the natural place to even it out.


🟡 Whitespace

Roughly 30 lines in §9 that read as blank in the diff actually contain a single space (SPEC.md:249, 251, 253, 255, …). Rendering is unaffected — CommonMark treats a space-only line as blank — but it adds noise to diffs and git blame on a document whose entire review workflow is diff-based. Worth stripping before merge.

💡 CI suggestion (optional, out of scope)

.github/workflows/ currently holds only the Claude review actions — no markdown linting. For a spec repo, a markdownlint + link-check (e.g. lychee) job on PRs would have caught the broken blockquote, the trailing whitespace, and the empty [previous Meeting Minutes]() link automatically. Happy to open a separate issue for this.


Summary

Nothing here blocks on substance — the numbers are right, and this PR quietly fixes a 1000× reporting error plus a self-contradictory value. Recommended before merge:

  1. Revert the stray blank line at SPEC.md:229 (rendering regression).
  2. Split out agenda.md and README.md, or fix the issues listed above.
  3. Restore the markdown link in the README rather than a bare @mention.
  4. Add the missing Inception/Deployment/Retirement bullets to §9.1.2's method list.
  5. Strip the space-only lines.

Items 4–5 and the ISO-consistency notes could reasonably be follow-ups if the group wants this section landed now. No security concerns — documentation-only changes, no executable content.

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.

4 participants