Skip to content

Revised the Functional units to comply with iso - #141

Open
seanmcilroy29 wants to merge 10 commits into
devfrom
Functional-units
Open

Revised the Functional units to comply with iso#141
seanmcilroy29 wants to merge 10 commits into
devfrom
Functional-units

Conversation

@seanmcilroy29

Copy link
Copy Markdown
Collaborator

Revised the Functional units 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 Functional units to comply with ISO

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

claude Bot commented Jul 14, 2026

Copy link
Copy Markdown

Review — PR #141: Revised the Functional Units to comply with ISO

Thanks for this. The ISO-style cleanup of section 8 (numbered tables with captions, NOTE/EXAMPLE labels, semicolon-delimited lists) is a genuine improvement in presentation. A few things below are worth addressing before merge — the first two are substantive, the rest are quality/process.

Normative language: inconsistency + one unintended strengthening

  1. Mixed keyword casing across the document. Section 8 now lowercases the normative keywords (shall, should, may), but section 7 and the rest of SPEC.md still use the RFC 2119 uppercase form (SHALL, MAY at lines 116-178). ISO/IEC Directives do use lowercase, so the intent is fine — but the doc now mixes both conventions, which is ambiguous for a normative spec. Recommend either (a) lowercasing doc-wide in one pass, or (b) keeping uppercase and adding a "Terms/Conventions" clause defining the keywords. Right now there is no clause in SPEC.md that defines requirement levels at all.

  2. MAY -> should is a semantic change, not just formatting. The old text read "Reporting multiple functional units MAY be encouraged" and became "Providers should report multiple functional units where feasible" (line 234). That promotes an optional allowance to a recommendation — a real change in conformance strength. If intended, please call it out in the PR description (and check it against FAQ.md); if not, revert to may.

Markdown rendering defects in section 8.2

  1. Missing blank line before the #### 8.2.2 heading (lines 220-221). The heading immediately follows a list item with no blank line, which can prevent it from rendering as a heading.

  2. "Gross values..." paragraph is glued to the bullet list (lines 227-228). There is no blank line between the last - ...synthetic data use). bullet and the Gross values refer to... paragraph, so it renders as a lazy continuation of the final bullet rather than its own paragraph — the gross/effective definition (previously a distinct blockquote) gets swallowed into a list item. Add a blank line. The previous blockquote styling that set off this definition was also dropped; consider whether that emphasis should be preserved.

  3. Trailing whitespace on the "blank" separator lines throughout section 8 (each blank line is actually a single space). Harmless to renderers but adds whitespace noise to the diff; recommend cleaning up.

Process (per CLAUDE.md)

  1. FAQ.md was not updated. CLAUDE.md requires SPEC changes be reconciled with FAQ.md. FAQ.md contains its own copy of the consumer functional-units table (~line 119) and discussion of multi-metric reporting — please confirm the reworded section 8 (and the may->should change in item 2) does not now diverge from or contradict the FAQ, and update it if it does.

  2. Dropped multi-metric example detail. The prior version listed the three example values as bullets; the new single-sentence EXAMPLE is more ISO-conformant but denser. Please double-check the numbers/units carried over correctly (0.45 gCO2e / 10^12 FLOPs, 0.18 / 1,000 tokens, 20 kgCO2e / billion parameters).

Scope / unrelated changes

  1. Two changes appear unrelated to "Functional Units":
    • README.md converts the maintainer entries from markdown links to plain text with handles (e.g. [Navveen Balani](https://github.com/navveenb) -> Navveen Balani (Accenture) @navveenb), dropping the clickable profile links — likely a regression. Adding Henry Richardson is fine.
    • .github/ISSUE_TEMPLATE/agenda.md is a new meeting-agenda template unrelated to this PR, so it would be cleaner as its own PR. It also (a) starts with ----/--- instead of valid GitHub issue-template YAML front matter (name:/about:), so it will not surface properly in the template chooser, and (b) overlaps with the existing agenda_minutes_template.md.

Nice

  • Table captions (Table 1, Table 2) and NOTE/EXAMPLE labels are the right ISO idiom.
  • Adding "pre-training, mid-training and post-training" and "intermediate & test runs" to section 7.2 usefully tightens the training-emissions scope (note & vs. spelled-out "and" — ISO prose usually avoids the ampersand).

Overall: the section 8 restructuring is a good direction. I would recommend splitting out the README/agenda changes, fixing the two rendering defects, resolving the keyword-casing convention, and either confirming or reverting the may->should change with a matching FAQ check.


Automated review by Claude Code

Clarified the reference to table 2 in the provider functional units section.

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

claude Bot commented Jul 31, 2026

Copy link
Copy Markdown

Review — PR #141: Revised the Functional units to comply with ISO

Thanks for pushing the ISO editorial alignment forward. The clause-8 restructuring (numbered/captioned tables, NOTE/EXAMPLE labels, semicolon-delimited requirement lists, sentence-case headings) is the right direction and matches ISO/IEC Directives Part 2 drafting style. Feedback below, ordered by impact. Adapting the usual review dimensions to a spec repo: "bugs" = rendering/normative defects, "tests" = lint/traceability.


1. Markdown rendering defect — SPEC.md:229-230 (blocking)

- any key strategies, assumptions, or methodologies that are ... (e.g., pruning, sparse activation, synthetic data use).
Gross values refer to total quantities without adjustment — e.g., total parameters ...

There is no blank line between the last bullet and the Gross values … paragraph, so CommonMark/GFM treats it as a lazy continuation of the third list item. It will render as part of that bullet, not as a standalone paragraph — which matters because 8.2.2 bullet 2 (gross or effective values (see below)) points at it.

Same class of issue at SPEC.md:222-223: #### 8.2.2 Reporting expectations immediately follows a bullet with no blank line (it renders, but it breaks the blanks-around-headings convention used everywhere else in the file).

2. Normative conflict introduced in 8.2

  • SPEC.md:208: "Provider functional units shall align with one of the following metrics"
  • SPEC.md:236: "Providers should report multiple functional units where feasible"

As written, a recommendation instructs providers to do something the requirement restricts. The previous wording (Reporting multiple functional units MAY be encouraged) was permissive, so the conflict was latent; converting it to should makes it live. Suggested fix: shall align with at least one of the metrics given in Table 2.

Also note this is a normative strengthening, not an editorial change (MAYshould = permission → recommendation). Per CLAUDE.md that needs FAQ coverage, and probably a WG decision on the call rather than riding along in an editorial PR.

3. The NOTE in 8.1 carries a recommendation

SPEC.md:204 — the NOTE says "emissions should account for all triggered operations". ISO/IEC Directives Part 2 requires notes to be informative only; requirements and recommendations are not permitted in notes. This is arguably the most substantive ISO-compliance gap left in the clause. Since multi-call/agentic accounting is genuinely normative, promote it into the body text of 8.1.

Related: gross and effective values are load-bearing for the shall in 8.2.2 but are defined only in running prose. Consider adding them as T.9/T.10 in clause 4.

4. Verbal-form conventions are now undefined document-wide

Clause 8 is lowercase shall/should/may; clauses 6 and 7 are still SHALL/MAY (SPEC.md:118, 132, 156, 160, 168, 172, 176, 180). Two knock-on points:

  • The document has no clause stating how verbal forms are to be interpreted. Uppercase keywords implied RFC 2119; moving to ISO lowercase without adding a verbal-forms statement (or citing ISO/IEC Directives Part 2 in clause 3) leaves shall formally undefined.
  • Sibling PRs Revised the Terms and definitions to comply with ISO #137Revised the Implementation examples to comply with iso #142 each convert a different clause against dev, so whichever lands first leaves dev half-converted. A tracking issue or an agreed merge order would help, plus a final consistency sweep — heading case is mixed now too (sentence-case ## 8. Functional units vs title-case ## 9. Implementation Examples), and so is table referencing (Table 1 provides at line 190 vs in table 2 at line 208; ISO capitalizes references to numbered elements).

5. Clause 7.2 changes

  • SPEC.md:168: intermediate & test runs& for "and" runs against the ISO style this PR is applying (the same PR correctly replaces & with and in Table 2). Please use and.
  • SPEC.md:163: pre-training, mid-training and post-training — none of the three are defined in clause 4, and mid-training has no settled industry definition. Either define them or use phrasing that does not depend on contested vocabulary. Also worth confirming the WG intends post-training (fine-tuning/RLHF) to sit in Design and Development, and what happens when a Consumer fine-tunes — FAQ.md is silent on both.

6. The EXAMPLE figures do not hold up (inherited, but this line is being rewritten here)

SPEC.md:238: 0.45 gCO₂e per 10¹² FLOPs, 0.18 gCO₂e per 1,000 training tokens, and 20 kgCO₂e per billion parameters.

  • 20 kgCO₂e / billion parameters is 1000× below FAQ.md:164, which puts GPT-4 at ~20 metric tons CO₂e per billion parameters. The matching numeral suggests a kg/t slip.
  • The three figures cannot describe a single training run. Solving them simultaneously with F ≈ 6ND implies ~7.4M training tokens and ~67M parameters — no plausible language model satisfies all three at once.

Since the EXAMPLE now asserts these come from one organization's model, this is a good moment to correct the numbers or state that they are unrelated illustrations.

7. Scope and PR hygiene

The title says "Functional units", but the PR also carries a README change and a new issue template. Splitting them would make the ISO work reviewable on its own. On those files specifically:

  • .github/ISSUE_TEMPLATE/agenda.md is a near-duplicate of the existing agenda_minutes_template.md. diff shows they differ only in the year, hardcoded chair names, *** vs blank separators, and a few dropped sections (Previous Meeting Action Items, Weekly Reminders, AOB). Two competing agenda templates will drift — prefer editing the existing one.
  • - [x] DD MMM and - [x] Motion to adjourn ship pre-checked; the existing template leaves them unchecked, which is correct for a template.
  • The hardcoded 2025.mm.dd placeholder is already stale.
  • No YAML frontmatter (name/about), unlike motion-to-approve.md. Optional, but it gives the template a readable label in the chooser.
  • The template embeds a personal email address into every issue created from it. A role alias or just the "open an Issue" link would age better.
  • Name spellings disagree across the two files: Naveen Balani / Henry Richards (agenda) vs Navveen Balani / Henry Richardson (README).
  • README.md:17 — replacing [Navveen Balani](https://github.com/navveenb) with plain @navveenb loses the link: GitHub does not autolink @mentions in repository markdown files (only in issues/PRs/comments). Suggest [Navveen Balani (Accenture)](https://github.com/navveenb) and [Henry Richardson (WattTime)](https://github.com/Henry-WattTime).

8. Nits and tooling

  • Every blank line added in clause 8 contains a single space character. Harmless when rendered, but it defeats trailing-whitespace tooling and adds noise to future diffs.
  • No FAQ.md changes in this PR. Items 2, 5, and 6 are the ones CLAUDE.md would flag as needing FAQ coverage. While in that area: FAQ.md:126 still lists Video Generation as Per Second or Per Frame for Generation where SPEC Table 1 says Per Second — worth aligning the two tables.
  • Suggested "test coverage": the only workflows here are the Claude ones. A markdownlint job (MD022 blanks-around-headings, MD032 blanks-around-lists, MD009 trailing spaces) would have caught items 1 and 8 automatically, and would be cheap insurance across the whole Revised the Terms and definitions to comply with ISO #137Revised the Implementation examples to comply with iso #142 series.

No security concerns beyond the email address noted above; there is no executable code in this diff.


Summary: item 1 should be fixed before merge (it changes how the clause renders), and items 2–3 are the ones keeping the clause from being ISO-conformant. The rest is consistency and hygiene. Happy to re-review once the rendering issue and the shall/should conflict are resolved.

🤖 Generated with Claude Code

@claude

claude Bot commented Jul 31, 2026

Copy link
Copy Markdown

Review: Revised the Functional units to comply with ISO

Thanks for pushing the ISO-alignment work forward — the restructure of §8 (numbered tables with Table 1 — / Table 2 — captions, NOTE/EXAMPLE paragraph markers, semicolon-delimited requirement lists) is the right shape for ISO/IEC Directives Part 2 style, and folding the multi-metric block into a single EXAMPLE paragraph is a genuine improvement.

A few things need attention before merge. Per CLAUDE.md, I've weighted anything that changes normative meaning or diverges from FAQ.md most heavily.


1. Markdown rendering bug in §8.2.2 (SPEC.md:230-231)

- any key strategies, assumptions, or methodologies that are ... (e.g., pruning, sparse activation, synthetic data use).
Gross values refer to total quantities without adjustment — e.g., total parameters ...

There's no blank line between the last bullet and the Gross values ... paragraph. Under CommonMark this is a lazy continuation: the gross/effective explanation renders as part of the third bullet rather than as its own paragraph. That's especially unfortunate because the second bullet says "(see below)" and the thing it points at is now swallowed into a sibling bullet.

Please insert a blank line:

- any key strategies, assumptions, or methodologies that are ... (e.g., pruning, sparse activation, synthetic data use).

Gross values refer to total quantities without adjustment — e.g., ...

2. FAQ.md is not updated, and this PR makes changes that need it

CLAUDE.md is explicit: "If your SPEC.md change contradicts or invalidates something in FAQ.md, the PR must include updates to both files" and "For substantial SPEC.md changes without existing FAQ coverage, add corresponding FAQ entries." Two changes here trip that rule:

(a) SPEC.md:163 — new training-stage scope. model training became model training stages (including, but not limited to, pre-training, mid-training and post-training). That is a substantive scope expansion, not an editorial one, and FAQ.md has zero coverage of it (grep -i "post-training\|fine-tun" over FAQ.md returns nothing). It also raises a boundary question the spec currently can't answer: §6.1 Consumer Boundary lists no training or fine-tuning component, so if a Consumer post-trains an off-the-shelf model, those emissions land in neither boundary. Is the intent that fine-tuning makes the Consumer a Provider for the derived model? That belongs in the FAQ either way.

(b) The MAYshould change at SPEC.md:237. See item 4.

3. Undefined terms: pre-training, mid-training, post-training (SPEC.md:163)

These appear in a SHALL clause but are defined neither in §4 Terms and Definitions nor in ISO/IEC 21031:2024 (the sole normative reference in §3). mid-training in particular is emerging jargon with no settled meaning across the industry. ISO style requires normatively-used terms to be defined or drawn from a referenced source. Either add §4 entries, or fall back to wording that doesn't depend on the taxonomy — the existing including but not limited to in the next sentence may already cover the intent.

4. MAYshould is a normative strengthening, not a style edit

Before:

Reporting multiple functional units MAY be encouraged, especially when feasible ...

After:

Providers should report multiple functional units where feasible ...

MAY (permission) → should (recommendation) is a real change in conformance obligation, and it's buried in a PR described as ISO compliance formatting. It's arguably the better requirement, but it should be called out in the PR description so reviewers on the weekly call can weigh in deliberately, and it needs FAQ rationale.

5. Normative keyword casing is now mixed document-wide

§8 now uses lowercase shall/should (ISO convention), but the rest of SPEC.md still uses uppercase RFC 2119-style keywords:

  • SPEC.md:118, 132, 156, 160, 168, 172, 176, 180SHALL / MAY
  • SPEC.md:186, 204, 208, 226, 237shall / should

Notably SPEC.md:168 in this very PR was edited and left as SHALL, two lines above the section that was converted to lowercase. I realize #137#142 are splitting this work section-by-section, so the mixed state may be transient — but mixed casing of conformance keywords is exactly the kind of ambiguity ISO review flags, and there's no tracking issue linked. Two suggestions: (a) note the intended end state in the PR description, and (b) whichever PR lands last should add a §3-adjacent statement of the conformance-keyword convention, since the doc currently declares one nowhere.

6. .github/ISSUE_TEMPLATE/agenda.md is unrelated scope creep, and has problems of its own

This new 51-line file has nothing to do with functional units. Suggest splitting it into its own PR. Issues regardless:

  • Malformed front matter. The file opens with ---- (four dashes) and closes with --- (trailing space), so it is not valid YAML front matter and carries no name: / about: / labels: keys — it won't get a proper label in the issue-template chooser. This matches the existing broken agenda_minutes_template.md, so it's consistent-with-repo rather than novel, but it propagates the bug. Compare motion-to-approve.md, which does it correctly.
  • Near-duplicate of an existing template. diff .github/ISSUE_TEMPLATE/agenda.md .github/ISSUE_TEMPLATE/agenda_minutes_template.md shows they're the same document with different placeholders. Two agenda templates in the chooser will confuse conveners. Prefer editing agenda_minutes_template.md in place, or delete it in this PR.
  • Pre-checked boxes. - [x] DD MMM and - [x] Motion to adjourn ship pre-ticked. A template should start unchecked (the existing template has these as - [ ]).
  • Name inconsistencies inside this same PR. agenda.md:6-7 says "Henry Richards" and "Naveen Balani", while README.md:17-18 in the same diff says "Henry Richardson" and "Navveen Balani". At least two of these four are wrong — worth getting right, these are real people's names.
  • Hardcoded 2025.mm.dd will need an annual edit, and is already stale.

7. README.md:17 loses a working link

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

Adding Henry is good. But GitHub does not autolink @mentions in repository Markdown files the way it does in issues and PR comments — that autolinking is comment-context only. So this trades a clickable profile link for plain text. Suggest keeping the link form for both:

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

Nits

  • SPEC.md:168intermediate & test runs. This PR deliberately expands &and in Table 2 (Algorithmic & hardware efficiencyAlgorithmic and hardware efficiency), then introduces a new & eight lines earlier. Use and.
  • SPEC.md:208 — cross-reference casing. shown in table 2 (lowercase) vs Table 1 provides ... (capitalized) at line 188. ISO cross-references are capitalized; pick one and apply it consistently.
  • SPEC.md:208 — stray comma. "shall align with one of the following metrics shown in table 2**,** to normalize carbon emissions" — the comma separates the verb from its purpose clause. Drop it, or recast as "... shall align with one of the metrics in Table 2 in order to normalize ...".
  • Table 2 unit/description mismatch (pre-existing, but this section is being rewritten so it's a cheap fix): the unit is named Per Parameter while the description and the EXAMPLE both use per billion parameters. Either rename the unit to Per Billion Parameters or normalize the description.
  • Whitespace-only lines. The new §8 uses lines containing a single space (" ") rather than truly empty lines as paragraph separators. It renders fine and matches what earlier ISO PRs introduced in §3, but it's invisible noise that trips most markdown linters and makes future diffs messier.

Test coverage

There's no automated check on the one artifact this repo actually ships — .github/workflows/ contains only claude.yml and claude-code-review.yml. Given that this PR series is making high-volume formatting edits across SPEC.md, a lightweight markdownlint-cli2 job (plus a link checker for the ISO/OBP/Electropedia URLs in §3–§4) would have caught the §8.2.2 lazy-continuation bug and the whitespace-only lines automatically, and would protect the remaining PRs in the #137#142 series. Happy to open that as a separate PR if useful.


Summary: item 1 is a straightforward rendering fix; items 2–4 are the ones I'd want resolved before merge, since they change normative meaning without corresponding FAQ.md updates. Items 6–7 would be cleanest as a separate PR. The core §8 restructure itself looks good.

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