Repository navigation
sow/2026-Q3: SAM-BIM Q3 2026 updates - #3
Merged
Merged
Conversation
* docs: add approved B0-B6 benchmark programme implementation plan Engine-neutral TAS/OpenStudio benchmark plan (Revision 2, stakeholder-approved) placed under docs/benchmark/ as the umbrella for milestones B0-B6. B0's methodology/metrics/schema/tolerances/glossary docs will be added alongside it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs: require commit+push after every stage (multi-laptop continuity) Adds a Multi-laptop continuity rule to the mandatory Git rules (S13) and session discipline (S12): commit and push each feature branch at stage end, and resume by fetching/checking out the existing branch rather than recreating it. Michal works across several laptops. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs: define benchmark methodology and glossary Co-Authored-By: Codex <codex@openai.com> * docs: define benchmark metrics and units Co-Authored-By: Codex <codex@openai.com> * docs: specify benchmark schema and tolerances Co-Authored-By: Codex <codex@openai.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Codex <codex@openai.com>
* docs: clarify benchmark numeric serialization * feat: add engine-neutral benchmark schema * feat: add deterministic benchmark serialization * feat: add reusable benchmark CLI host * test: complete benchmark schema validation coverage * fix: harden benchmark input parsing * docs: align benchmark precision guidance * chore: add required SPDX headers
* feat: define canonical benchmark model hashing
B1a requires canonicalModelHash for successful benchmark documents, but the
merged schema defined no reproducible canonical SAM representation, so B1b
producers had nothing to call and would have had to invent engine-specific
hashing.
Add an engine-neutral canonicalization contract and implementation:
- BenchmarkCanonicalization: the single authoritative canonicalization version
("1.0.0") plus major/minor classification, reusing BenchmarkSchema version
parsing.
- BenchmarkCanonicalJson: canonical JSON v1 over UTF-8 bytes or text, and
SHA-256 of the canonical bytes through the existing BenchmarkHash format.
- CanonicalJsonWriter: internal recursive traversal that sorts object
properties in ordinal order, preserves array order, normalises numbers and
escapes, and rejects duplicate names, comments, trailing commas, non-finite
numbers and malformed text.
Validation now classifies canonicalizationVersion instead of only parsing it:
1.0.0 is accepted, an unsupported major version is rejected as
provenance.canonicalizationVersion.major, and malformed versions keep the
existing code. canonicalModelHash stays mandatory for successful documents and
failure documents are not made stricter.
The library gains no SAM, OpenStudio, TAS, Rhino or Grasshopper dependency: it
canonicalizes JSON a producer supplies. Obtaining the neutral SAM model JSON
from a loaded AnalyticalModel remains each producer's responsibility.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* docs: specify canonical benchmark model representation
B0 named canonicalization and B1a made canonicalModelHash mandatory, but
neither document said what the canonical representation actually is, so a
producer could not implement the hash without guessing.
SCHEMA.md gains a Canonical model hashing section: the sourceFileHash versus
canonicalModelHash distinction as a table, the six-step producer procedure,
the source-bytes-to-canonical-hash pipeline, the requirement to hash the
neutral loaded SAM model before any engine-specific translation or mutation,
the exact canonical JSON v1 rules, and a warning that changing any rule
changes every hash and requires a new canonicalizationVersion. The
compatibility policy now states what validation enforces: a well-formed
version on every document, and rejection of an unsupported major.
METHODOLOGY.md gains the same distinction and pipeline in workflow terms, and
states that producers call the shared helper rather than inventing their own
canonicalization, so a hash difference means a model difference.
The rules are documented as a versioned SAM benchmark contract, explicitly not
a claim of full semantic JSON equivalence, and no OpenStudio- or TAS-specific
implementation detail is introduced.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* build: output benchmark schema assembly to build\ Decision D3-build requires the schema project to be in SAM_Validation's solution and to output to SAM_Validation\build\. B1a delivered the first half but not the second: with no OutputPath the assembly landed in bin\Release\netstandard2.0\ and no build\ folder existed. B1b and B2 reference this assembly across repositories by HintPath, following the pattern SAM_OpenStudio already uses for SAM itself (..\..\..\SAM\build\SAM.Core.dll), so the output has to be a flat, unsuffixed folder. Without this the producers cannot resolve the reference at all. Set OutputPath to ..\build\ with AppendTargetFrameworkToOutputPath and AppendRuntimeIdentifierToOutputPath disabled, matching SAM\SAM\SAM.Core\SAM.Core.csproj. build\ is already covered by .gitignore, so the output stays untracked. Verified: Debug and Release both emit build\SAM.Analytical.Benchmark.dll with no framework subfolder; the solution builds clean; all 77 benchmark tests pass; and a net8.0 console exe referencing only the built DLL by HintPath runs without a System.Text.Json package reference, resolving it from the Microsoft.NETCore.App shared framework. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs: correct benchmark producer HintPath depth in plan The B1b/B2 HintPath in IMPLEMENTATION-PLAN.md read ..\..\SAM_Validation\build\SAM.Analytical.Benchmark.dll, one level short. The producer projects sit three directories deep in their repos (e.g. SAM_OpenStudio\benchmark\SAM.Analytical.OpenStudio.Benchmark\), the same depth as the existing ..\..\..\SAM\build\SAM.Core.dll references in SAM.Analytical.OpenStudio.csproj. The correct relative path therefore has three ..\ segments. Verified empirically: a net8.0 exe referencing the built DLL at ..\..\..\SAM_Validation\build\ compiles and runs. Corrected all three occurrences (dependency diagram, B1b, B2) and noted the reason inline so the producer sessions do not rediscover it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* feat: B3 benchmark comparator + reports (SAM.Analytical.Benchmark.Compare) Add the headless, engine-independent comparator for the TAS<->OpenStudio benchmark programme. It reads two neutral benchmark JSON documents, aligns and diffs them, assigns provisional tolerance bands, and emits deterministic Markdown + CSV + summary-JSON reports plus a gate status. It references ONLY the neutral schema library (no engine assembly), enforced by a CI reference-check. New project SAM.Analytical.Benchmark.Compare (net8.0, exe `benchmark-compare`): - Classes: ComparisonResult, MetricComparison, SpaceComparison, SpaceMatchDiagnostics, ReconciliationResult, ToleranceProfile, enums, SchemaIncompatibleException. - Query: Compare (schema-compat check + model/space diff + reconciliation), AlignSpaces (Guid-first, Name-fallback; duplicates/split diagnosed), Band (combined near-zero-floor + relative limit; either side unavailable or unit-mismatch => N/A, never a failure), CircularHourDiff (year-boundary wrap), GateStatus (worst applicable band), Reconcile (additive model-total vs sum-of-spaces). - Modify: WriteMarkdown/WriteCsv/WriteSummaryJson + WriteReports — deterministic, invariant culture, full finite double precision (no rounding), sorted keys, LF newlines, UTF-8 no BOM. comparison-summary.json stamps summarySchemaVersion=1.0.0. - Program: shared BenchmarkCliHost/BenchmarkArguments/BenchmarkCliPaths; exit codes 0/2/3/4/5, plus exit 1 only under --fail-on-gate. New MSTest project SAM.Analytical.Benchmark.Compare.Tests (44 tests, all portable, fabricated in-memory pairs): exact/warn/fail/beyond bands, near-zero, null/unavailable => N/A, unit-mismatch guard, missing/duplicate/split spaces, circular peak-hour, model-total vs sum reconciliation, schema-version drift, CLI exit codes, and a byte-stable golden of all three reports (regenerate with SAM_BENCHMARK_UPDATE_GOLDEN=1). Both projects added to SAM_Validation.sln. build.yml gains an engine-reference guard and a dotnet-test step for the comparator. .gitattributes pins the golden fixtures to eol=lf (repo uses core.autocrlf=true). Generated by Michal Dengusiak and CodeClaude Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix: align comparator with TOLERANCES.md contract (Codex review) Addresses the three P2 findings from the Codex review of PR #8, all grounded in the authoritative docs/benchmark/TOLERANCES.md contract, plus a related contract gap found while fixing them. 1. Banding scale (Band + Reconcile): the near-zero floor is the lower bound of the SCALE (denominator), not an additive allowance. Now scale = max(|tas|, |os|, nearZeroFloor(unit)) and the band is taken from relativeDifference = abs/scale against the strict boundaries (<5% Match, <15% Warn, else Fail). Previously `floor + relative*magnitude` false-passed near-zero disagreements (e.g. 0 W vs 10 W with a 10 W floor reported Match instead of Fail). 2. Persist tolerance values in reports: comparison-summary.json gains a `toleranceProfileDetail` object (warn/fail relative, hour warn/fail, default + per-unit near-zero floors) and comparison.md gains a "Tolerance profile" section, so a changed profile can never hide behind an unchanged name. 3. Reconcile only uniquely-matched spaces: reconciliation now sums only the spaces uniquely matched across both documents; one-sided/ambiguous spaces are excluded and reported (Used/Missing/Excluded) with a `complete` flag that is false whenever anything was left out. Also (contract, not flagged by Codex): peak-hour differences are informational and "cannot produce a numerical Fail" per TOLERANCES.md, so hour-of-year metrics are now excluded from GateStatus. Their Match/Warn/Fail bands are still shown in the reports for information. ComparisonResult now carries the full ToleranceProfile (not just its name). Tests updated: near-zero rewritten to the correct semantics + the TOLERANCES.md near-zero table reproduced; new tests for the peak-hour gate exclusion and reconciliation completeness. Goldens regenerated. 50/50 pass. Generated by Michal Dengusiak and CodeClaude Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix: enforce provenance + coverage gating, stricter alignment (Codex re-review) Addresses the "not ready to merge" Codex review of PR #8. The comparator now enforces the frozen B0 contracts it previously skipped, so it can no longer issue a reassuring Pass for an invalid comparison pair. Provenance compatibility (P1): Query.Compare now checks the two runs are actually comparable via a new ProvenanceCompatibility result — expected engine kind + route family on each CLI side, both state=Success, compatible canonicalization version, equal canonical model hash, equal weather hash/ identity, equal design-day source, equal source-model GUID, and native<->native / shared<->shared route pairing. Any mismatch makes ProvenanceStatus=Fail and prevents an overall Pass. Reported in both md and summary JSON. Multi-dimensional gate (P1): ComparisonResult now exposes NumericalStatus, CoverageStatus, ProvenanceStatus and ReconciliationStatus plus an overall Gate = worst of the four. Unmatched/ambiguous/duplicate spaces or zero comparable metrics make coverage incomplete (Warn); an incomplete/out-of-band reconciliation is Warn; provenance incompatibility is Fail. Missing data can no longer present as a clean pass. Compare self-validates (contract errors): Query.Compare validates both documents with BenchmarkValidator, so a wrong unit, broken availability invariant, out-of-range hour, or malformed/major-incompatible schema is a contract error (BenchmarkValidationException -> exit 4) rather than a silently-normalised N/A. Removed the bespoke SchemaIncompatibleException. Stricter alignment: a duplicated GUID now excludes every member of that group from matching (no arbitrary first-match); name fallback runs only when at least one side lacks a usable GUID (two different valid GUIDs are never merged by name); one-sided outputs are explicitly sorted for determinism. Peak-hour band capped at Warn (never Fail) and still excluded from the numerical gate, per the contract's "informational" rule. Schema-drift note now warns on a minor drift (including equal-but-newer minors) but not on a patch-only difference. Packaging: benchmark-compare now outputs to the repo build\ folder so CI's `**\build\*` artifact upload includes the executable, matching the producers. Tests updated + new coverage: provenance mismatch (weather / swapped engine), duplicate-GUID exclusion, GUID-conflict-no-name-merge, equal-newer-minor drift, patch-only no-drift, unit-mismatch-is-contract-error, coverage-incomplete gate. Goldens regenerated. 57/57 pass. Generated by Michal Dengusiak and CodeClaude Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix: reject unknown CLI flags; keep gate/caveat wording accurate (Codex) Addresses the three P2s from Codex's third review of PR #8: - Reject misspelled optional flags: Program validates every supplied option against the comparator's known set (tas, openstudio, out, tolerance-profile, fail-on-gate) and returns a usage error (exit 2) for anything else, so a typo like `--fail-on-gtae` can no longer be silently ignored and disable gate enforcement. - Correct the reconciliation description in comparison.md: it is a separate within-document diagnostic status that ALSO contributes to the overall gate (the overall gate is the worst of numerical/coverage/provenance/reconciliation), instead of claiming it is kept separate. - Keep methodological caveats visible: the model-metrics section now prints the whole-model peak-load semantics caveat (OpenStudio coincident-total vs TAS building-profile max) and the candidate conditioning-pairing limitation even when values fall in Pass, as TOLERANCES.md/METRICS.md require. New CLI test for the misspelled-flag rejection. Goldens regenerated. 58/58 pass. Generated by Michal Dengusiak and CodeClaude Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs: record first real cross-engine comparison (same model + same weather) Runs the B3 comparator end-to-end over two real engine documents produced from the SAME source model and the SAME weather file — the numeric pairing the B2/D10 checkpoint had to defer (it used a geographically mismatched Boston EPW). Run: HungaryHouse-WeahterDDY.sam (14 spaces, embedded design days) + HUN_CEN_Budapest.Met.Center.128400_TMYx.epw, Tas 9.5.7.0 and EnergyPlus 25.1.0 on the licensed workstation. Both producers exited 0 with state=Success; the comparator wrote all three reports. Verbatim artefacts committed alongside. Comparator validated on real data: 14/14 spaces matched by GUID (coverage Pass); floor area agrees to a 2.3e-7 relative difference between the two engines; reconciliation complete 14/0/0 and Match for both engines; the circular peak-hour rule correctly reported hour 0 vs 7974 as 786 h apart; 66 unavailable metrics all N/A without contributing a failure; output deterministic. Finding 1 — the provenance gate caught a real producer defect. It reported INCOMPATIBLE on designDaySource (TAS EmbeddedModel vs OpenStudio None). That is not a genuine input difference: the OpenStudio producer hardcodes DesignDaySource.None (SAM_OpenStudio .../Program.cs:86) instead of deriving it. The gate did exactly its job. Fix belongs in SAM_OpenStudio, separate PR; nothing to change in B3. Finding 2 — the D10 candidate conditioning pairing is NOT yet numerically equivalent: annual heating 4331.96 vs 1751.18 kWh (59.6%), peak heating 3.934 vs 5.721 kW (31.2%), per-space peaks 29-52%. It must stay labelled a candidate; the design-day defect is a plausible contributor, so re-assess after that fix and against the B4 corpus rather than by tuning tolerances. Also noted: TAS emits peakCoolingHour=0 available for a measured-zero cooling load, and the engines disagree on which spaces are conditioned. .gitattributes pins docs/benchmark/reports/** to eol=lf so recorded artefacts stay byte-exact under core.autocrlf. Generated by Michal Dengusiak and CodeClaude Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix: count unavailable required metrics in coverage (Codex R5) Codex reviewed 9ff04ca and found that partial metric coverage could still report CoverageStatus=Pass: the check only asked whether the aggregate comparable count was non-zero, so a run where a required metric was N/A on one side — while spaces aligned and reconciliation completed — produced an overall Pass despite missing results. N/A metrics do not affect the numerical status either, so nothing else caught the gap. TOLERANCES.md requires unavailable required metrics to count towards coverage precisely so poor coverage cannot appear complete. Coverage is now Warn when any required metric is unavailable, any space is unmatched/duplicated/ambiguous, or nothing was comparable at all. "Required" means the whole-model metrics plus the metrics of uniquely matched spaces: one-sided and ambiguous spaces are deliberately excluded, because every one of their metrics is N/A by construction and that gap is already reported by the unmatched-space diagnostics — counting them again would double-count it. - ComparisonResult exposes RequiredMetrics/RequiredMetricCount and UnavailableRequiredMetricCount; ComparableMetricCount is now scoped to the same comparable set (same value as before, since one-sided metrics were always N/A). - comparison-summary.json gains a "coverage" object: status, requiredMetricCount, comparableMetricCount, unavailableRequiredMetricCount plus unmatched/ambiguous/ duplicate space counts. Additive; summarySchemaVersion stays 1.0.0 (never shipped). - comparison.md gains a "## Coverage" section spelling out the counts and every reason coverage is incomplete, and two rows in the summary table. Two further findings from the same review round, both doc-backed: - Band applied only HourWarnAbsolute, so a 100 h peak-hour gap was reported identically to a 10 h gap and the configured 24 h fail threshold was dead. TOLERANCES.md requires both absolute hour thresholds. Peak-hour differences stay informational the way they already were — hour-of-year metrics are excluded from NumericalStatus — rather than by suppressing the reporting band. - CheckProvenanceCompatibility compared canonicalModelHash even when the two documents declared different canonicalizationVersions. SCHEMA.md says hashes from different canonicalizers must not be compared as equivalent evidence and that a version mismatch does not prove different models, so the hash check is now skipped in that case; the version mismatch remains the reported finding. Corrected the stale additive-limit wording (floor + relative * max) left on ToleranceProfile, Band and BandTests: the floor bounds the SCALE (max(|tas|,|os|,floor)) and the band comes from the relative difference. Tests: 65 comparator + 77 schema pass. New coverage cases for an unavailable whole-model metric, an unavailable metric in a uniquely matched space, everything available, nothing comparable, and one-sided space metrics not being double-counted; plus the two provenance canonicalization cases and the peak-hour fail band. The test that asserted an unavailable model metric preserved an overall Pass now expects Numerical=Pass, Coverage=Warn, Gate=Warn. Goldens regenerated. docs/benchmark/reports/2026-07-25-cross-engine-smoke reports regenerated from the same committed neutral inputs (no re-simulation, no input byte changed): that comparison now correctly reports Coverage=Warn for its 66 N/A metrics; its overall gate stays Fail on the existing numerical and provenance findings. TOLERANCES.md states the coverage and peak-hour rules explicitly. Generated by Michal Dengusiak and CodeClaude Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: exclude only the whole-model peak loads from the numerical gate (Codex P1) Adopts the remaining Codex P1 narrowly, per Michal's decision. METRICS.md keeps the whole-model peaks informational because OpenStudio reports a coincident total while TAS takes the maximum of the building profile, so a difference there can reflect aggregation semantics rather than engine physics — it should not fail an experiment. Excluded from NumericalStatus: model.peakHeatingLoad and model.peakCoolingLoad only. Still gating: annual heating/cooling, per-space peak and design loads, unmet hours, and all geometry. hourOfYear metrics stay excluded as before. The exclusion is deliberately narrow — the same "differing semantics" argument would otherwise extend to the candidate conditioning pairing (annual energy) and leave nothing to gate on. - New explicit predicate Query.IsInformationalForNumericalGate(metric): hour-of-year by unit, plus the two whole-model peak-load keys held in one set, matched by scope + key. Query.ModelScope replaces the repeated "model" literal in CompareModel, so there are no scattered string checks. Per-space peaks use the distinct keys heating.peakLoad/cooling.peakLoad, so a space NAMED "model" cannot be captured. - NumericalStatus now skips whatever that predicate designates. Informational status is expressed by exclusion, never by hiding evidence. The bands are still calculated, still counted in the report totals, and now marked on the row itself: Format.MetricNote appends "informational (excluded from the numerical status)" to any metric that carries a real band but cannot move the gate (Markdown + CSV note column), and comparison-summary.json gains an explicit per-metric "informational" boolean so a downstream consumer never has to infer why a Fail band did not gate. The Markdown intro and the model-metrics caveat now name the two excluded metrics and state that everything else gates normally. Docs reconciled: TOLERANCES.md gate aggregation gains an "Informational metrics" section (the two designated families, why the designation is narrow, and that promotion into the gate needs the B4 corpus plus energy-modeller review); METRICS.md's whole-model peak caveat now says concretely which two keys are excluded and that per-space peaks are not. Tests: 86 comparator + 77 schema pass. New cases prove a failing whole-model peak load alone leaves NumericalStatus=Pass while its band stays Fail and is still counted; a failing annual-energy metric still fails; a failing per-space peak load still fails; unmet hours and geometry still fail; and a mixed case takes the worst NON-informational band (informational Fails present, gating Warn -> Warn). A 14-row data test pins exactly which scope/key/unit combinations are informational. Two existing tests that used a whole-model peak load as their failing/warning metric now use annual energy, which is what they were actually asserting about. Goldens and the recorded cross-engine reports regenerated from the same committed neutral inputs — no re-simulation, no input byte changed. Worth recording: narrowing the exclusion did NOT rescue that comparison. Its numerical status is still Fail on the gating metrics alone (annual heating 59.6%, per-space peaks and unmet hours), with the peak loads now visibly marked informational. The B4 corpus and energy-modeller review still own any promotion of these metrics into the gate. Generated by Michal Dengusiak and CodeClaude Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: keep the CI SAMVersion when running the comparator tests (Codex) Codex reviewed 4eb04d2 and found that the `dotnet test` step this PR added silently un-stamps the uploaded assemblies. Confirmed by reproduction, not just by reading: dotnet test ... -c Release /p:SAMVersion=2026.3.9999.0 -> build\benchmark-compare.dll = 2026.3.9999.0 build\SAM.Analytical.Benchmark.dll = 2026.3.9999.0 dotnet test ... -c Release (as the workflow had it) -> build\benchmark-compare.dll = 1.0.0.0 build\SAM.Analytical.Benchmark.dll = 1.0.0.0 The test project references the schema and comparator projects, and both output to the shared flat `build\` folder, so `dotnet test` rebuilds them straight into the folder the msbuild step had just stamped. Without the property, Directory.Build.props falls back to its local-dev default 1.0.0.0 (line 5) and the correctly versioned assemblies are overwritten immediately before `Upload build folders` uploads `**\build\*` — so every CI artefact from this workflow would have carried 1.0.0.0. Fixed by passing the computed `/p:SAMVersion` to the test step as well, with a comment explaining why it is not optional. Preferred over `dotnet test --no-build`, which would depend on the msbuild and dotnet builds resolving identical output paths. Comparator tests still 86 pass under both invocations. Generated by Michal Dengusiak and CodeClaude Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: carry the tolerance profile and its values in comparison.csv (Codex) TOLERANCES.md:15 requires every report to name the tolerance profile AND include its actual values, so a changed profile cannot hide behind an unchanged name. The Markdown and summary JSON did; the CSV carried bands with no way to interpret or reproduce them. Codex flagged it on d355df7, and the CSV is precisely the artefact most likely to be archived or imported on its own. Seven columns appended (existing column positions unchanged): toleranceProfile, warnRelative, failRelative, hourWarnAbsolute, hourFailAbsolute, nearZeroFloor and informational. Repeated per row rather than written as a preamble, which would break strict RFC-4180 parsing. nearZeroFloor is the floor for THAT row's unit, so every row is independently reproducible: scale = max(|tas|, |openStudio|, nearZeroFloor), then band from warnRelative/failRelative — and the informational column says whether the row's band fed the numerical status, matching the marker already in the note column. New test asserts the header tail, the profile values on a kWh row (floor 1) and a W row (floor 10), and that no row omits the profile. 87 comparator + 77 schema pass. Goldens and the recorded cross-engine CSV regenerated from the same committed inputs; no re-simulation. Generated by Michal Dengusiak and CodeClaude Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…ucer (#9) Repeats the same-model/same-weather run now that SAM_OpenStudio #17 (1ae8637) derives designDaySource from the route instead of hardcoding None. Same licensed workstation, same model (HungaryHouse-WeahterDDY.sam, 14 spaces, embedded design days) and same weather (HUN_CEN_Budapest TMYx). Producers built from SAM_Tas@ff00497 and SAM_OpenStudio@1ae8637; comparator SAM_Validation@c3eec2cafaf42267a506579a9efd078cb7fcc473 (the B3 squash merge). Tas 9.5.7.0 (60.8 s) and EnergyPlus 25.1.0 / SDK 3.10.0 (21.5 s), both exit 0 state=Success; comparator exit 0. Result 1 — the provenance gate now PASSES. OpenStudio reports designDaySource=EmbeddedModel (was None), matching TAS, and the two documents agree on canonicalModelHash, sourceModelGuid and the weather hash. ProvenanceStatus Fail -> Pass. The defect the gate caught is closed and the fix is verified end to end. Result 2 — not one simulated number changed. The complete generated comparison.csv is byte-identical to the 2026-07-25-cross-engine-smoke run: both files hash to the same Git blob 8406556, so every value, difference, band, note and tolerance column agrees exactly. (comparison.md and comparison-summary.json differ, as they must — they carry the provenance block, which is what changed.) That confirms the defect was exactly what it was diagnosed as, a REPORTING defect: the route was already building design days from the embedded model. It also doubles as a reproducibility check — two independent full runs of both engines produced identical metrics to the last binary64 digit, across TAS COM, a gbXML round trip, EnergyPlus and both producers' extraction paths. Result 3 — the candidate conditioning pairing is still NOT equivalent. The overall gate stays Fail, now on the numerical status alone: annual heating 4331.96 vs 1751.18 kWh (59.6%). Among comparable per-space heating peak-load rows, relative differences span approximately 12-100% across the 11 comparable rows; the 29% and 52% figures quoted earlier are selected examples, not the complete range. Previously that Fail was confounded by the provenance mismatch, so the disagreement could be set aside as "probably the declared design-day basis"; that specific objection is now answered. The two whole-model peak loads are banded and shown but excluded from the numerical status, so they are not what fails it. The D10 pairing must stay labelled a candidate and be investigated metric by metric against the B4 corpus — not by adjusting tolerances. What this run does NOT establish: it does not eliminate design-day handling as a cause of the load gap. It shows both runs declare the same coarse designDaySource CATEGORY and that a provenance-only fix changed no result; neither is evidence that the engines instantiated equivalent design days AFTER translation. canonicalModelHash describes the neutral loaded SAM model and is computed before any engine-specific modification (METHODOLOGY.md), and designDaySource records only Ddy/EmbeddedModel/None. Differing design-day translation therefore remains an open and leading hypothesis — TAS sizes through TBD while OpenStudio emits SizingPeriod:DesignDay objects — and closing it needs engine-level evidence comparing the design days actually instantiated on each side. Recorded as such in the README rather than claimed shut. Carried forward for B4, with the 66 unavailable required metrics now broken down in the README: OpenStudio's missing per-space design loads account for 28 of the 66 (heating+cooling designLoad across all 14 matched spaces) and are the largest single source of incomplete coverage; a further 24 are cooling peakLoad/peakHour on 12 spaces where TAS reports no cooling result but OpenStudio does. TAS still reports peakCoolingHour=0 available for a measured-zero cooling load, and the engines disagree about which spaces are conditioned — a bounded lead worth chasing first. Artefacts are the verbatim outputs: two neutral inputs plus the three generated reports, LF-pinned by the existing docs/benchmark/reports/** gitattributes rule. No simulation was re-run for these corrections and no neutral benchmark JSON was modified. Generated by Michal Dengusiak and CodeClaude Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* docs: record the cross-engine run with per-space design loads Third run in the benchmark report series, on the same model, weather bytes and workstation as the two 2026-07-25 runs, now with SAM_OpenStudio #18 (design-day pressure), #19 (diagnostic propagation) and #20 (per-space design loads from ZoneSizes) merged. Coverage rises 82 -> 92 of 148 comparable. comparison.csv differs from the previous run in exactly 10 rows, every one a heating.designLoad moving from TasOnly to Both; the other 138 rows are byte-identical, so no simulated value moved. The 10 recovered rows disagree in both directions - five above TAS, five below, aggregate OS/TAS 1.396 - which rules out a single multiplicative sizing margin as the explanation and answers the open question about whether TBD's maxHeatingLoad is pre- or post-factor. 68 conversion diagnostics now reach the artefact where the previous run recorded none. They name the London design days carried by this Budapest model, the constant-dew-point and ASHRAEClearSky approximations that replace the hourly design-day profiles (clearness 0.0 on the heating day), and two apertures skipped as degenerate. Two inert provenance deviations are recorded rather than hidden: the weather identity string differs because it is the file name and the earlier runs used a copy named Budapest.epw (the hash is unchanged), and samCommit is an unverifiable operator claim, so the SAM binaries actually loaded are pinned by SHA-256 instead. The merged 2026-07-25 record is left untouched as an immutable artefact. Generated by Michal Dengusiak and CodeClaude Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs: keep the TAS sizing-factor question open The bidirectional design-load spread rules out a uniform sizing margin as the SOLE explanation, but it cannot establish whether TBD's maxHeatingLoad is pre- or post-factor. That question is about the TAS side alone and needs a TAS design load compared against its own unfactored TBD result. The earlier wording rested on a premise this same report contradicts: a constant factor would move every row the same way only if the two engines were evaluating comparable physics. They are not - the design-day translation is lossy and the OpenStudio side drops two apertures - so a constant multiplier on the TAS loads remains consistent with ratios above 1 in some spaces and below 1 in others. Raised by Codex on PR #10. Generated by Michal Dengusiak and CodeClaude Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* docs: draft the design-day-audit-v1 neutral contract Defines design-day-audit-<engine>.json: an engine-neutral record of the design days each engine actually used and how each engine responded. Docs only - no schema types, no producers, no comparator, per the agreed sequence that the TAS and OpenStudio producer branches wait until this is settled. Built around the three representations, declared and never inferred: ObservedHourly (TAS, from TBD - nothing fitted), ProcessedParametric (EnergyPlus, echoed back in eplusout.eio - not a series) and DerivedHourly (reconstructed offline from those parameters). The rule that a DerivedHourly series was never "reported by" the engine is normative, and comparing a parametric definition hour-by-hour against an observed series is prohibited. Collapse loss - an ObservedHourly summarised into parametric terms against the engine's own echo - needs no reconstruction and is therefore the strongest available evidence. Stage A (conditions) and Stage B (sizing response) are structurally separate: separate top-level objects, separate statuses, joined only by designDayKey, with the rule that a Stage B difference is uninterpretable without its Stage A pairing. Provenance is stricter than the benchmark contract, because the recorded runs showed a samCommit whose code was demonstrably not in the loaded binaries: source files and hashes, LOADED ASSEMBLY HASHES as the authoritative record (commit strings are subordinate), per-design-day identity and location (per day, not per document - the recorded fixture carries London design days with Budapest weather), model vs weather-file elevations with an explicit elevationAuthority that may be Unknown, sizing factors with a required reportedLoadBasis, and a value basis of Observed / EngineEchoed / Reconstructed on every number. reportedLoadBasis is the field that makes the pre/post-sizing-factor question answerable, and the contract states it is settled per engine against that engine's own unfactored result - never by a cross-engine ratio. Validation makes an unlabelled or over-claimed value an error rather than a reporting judgement: a missing basis, an EngineEchoed series, a reconstruction without a declared algorithm, or an omitted basis field are all invalid, while Unknown is always a legal answer. Records what the TAS side already provides (DesignDataDictionary yields peak scalars and the owning design data) and three constraints on it: MaxValueDictionary collapses the per-design-day dimension, the GetPeakZoneGains index base and span are unverified so peakHour stays unavailable until pinned, and YearlyValues.cs must not be reused for the 24-hour series. Section 11 lists five open questions that need a decision before implementation. Generated by Michal Dengusiak and CodeClaude Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs: fix four contract defects found by Codex review All four were real gaps in the draft. P1 - cross-document alignment was undefined. `key` is only stable within one document and `name` is deliberately the unmodified engine label, so a translated or renamed design day had no deterministic way to be paired across the two documents, leaving both Stage A comparisons and the Stage B interpretation resting on them ambiguous. Added a required `alignmentKey` derived from the SHARED SAM SOURCE design day - the common ancestor - since SAM DesignDay derives from Weather.WeatherDay and carries no GUID: the key is `<LoadType>|<NormalisedSourceName>` with a specified normalisation (trim, collapse internal whitespace, ToUpperInvariant) and a versioned `alignmentKeyVersion`. Pairing is on exact equality only, with no fuzzy fallback; an untraceable day gets a null key and is reported one-sided rather than guessed; a duplicated key excludes the whole group from pairing, mirroring the duplicate-GUID space rule. P1 - Stage B series were invalid by construction. `HourlySeries.quantity` was restricted to the Stage A token list, so the promised TAS `zoneSensibleLoad`/`zoneTemperature` series would fail validation rule 4 whenever emitted. Added a Stage B token table (zoneSensibleLoad, zoneLatentLoad, zoneTemperature, zoneHumidityRatio, zoneSetpointTemperature) and scoped the rule to each stage's own list, with crossing prohibited. P2 - the parametric block declared every field an AuditValue, whose value is a number, while the contract then listed strings (dryBulbRangeModifierType, humidityConditionType, solarModel) and booleans. No implementation could satisfy both. Split into three shapes sharing the available/basis discipline: AuditValue (number, unit required), AuditToken (string, no unit) and AuditFlag (boolean, no unit), with tokens and flags compared for exact ordinal equality only. Named `dimensionless` as a canonical unit and applied it to clearness, tau and the sizing factors; ashraeCoefficientA is W/m2. P2 - the Stage B sort key was not total. One space yields a record per design day and load type, so guid+name left ties that would serialize in producer-enumeration order. Sort is now the total key (guid, name, designDayKey, loadType) with null designDayKey last - the same lesson already paid for in PR #20. Also added validation rules for unit/type correctness, a self-checking rule that `alignmentKey` must equal what its declared version's rules produce, a rule making non-total ordering a contract error, and open question 6 on whether name plus loadType is a strong enough key given the absent GUID. Generated by Michal Dengusiak and CodeClaude Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs: make the availability discipline consistent across all shapes Four more Codex findings, all real, and all consequences of the previous round's tightening. P1 - the new type rule rejected every unavailable field. Rules 1-2 require `value` to be null when unavailable, but rule 6 then rejected null because it is neither number, string nor boolean - invalidating every unavailable field, including the `peakHour` the contract explicitly instructs the TAS producer to emit as unavailable. Rule 6 now fires only on non-null values, and states that a null is governed by rule 1 alone. P1 - an unavailable HourlySeries was impossible to express. The shape required `basis` to stay `Observed` or `Reconstructed` while rule 2 rejects any non-null basis when unavailable, so the unavailable series that §7 instructs producers to emit could not satisfy the contract. Series `basis` is now nullable on exactly the same "if and only if unavailable" condition as `AuditValue`: an unavailable series keeps its `quantity` and `unit` and carries `basis: null`, `values: null`, so the declaration survives without claiming a basis for something that does not exist. P2 - a DDY run could not be recorded. `origin` may be `Ddy`, but no `sourceFiles` role identified the DDY file, and filing it under `Weather` would conflate two distinct inputs with distinct hashes. Added a `Ddy` role plus new rule 15 requiring it whenever `origin` is `Ddy`. P2 - `absoluteDifference` had no truthful basis. It is producer arithmetic over two `Observed` elevations, which is not `Observed`, not `EngineEchoed`, and not `Reconstructed` (reserved for modelling what the engine would have done from echoed inputs). Added a deliberately narrow `Calculated` basis for plain arithmetic over values already in the same document, explicitly not a softer synonym for `Reconstructed`, and new rule 16 pinning every `Calculated` value to its operands so it can never contradict them and must be unavailable when either operand is. This also covers the parametric summary floated in open question 5. Also corrected the parametric block, which asserted every field carries `EngineEchoed` - true only of available fields. Generated by Michal Dengusiak and CodeClaude Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs: specify the root document and make declarations enforceable Five more Codex findings, all real. P1 - the root document structure was never specified. Field-level rules alone would let the TAS and OpenStudio producers satisfy every one of them and still emit structurally incompatible JSON, because nothing said where sourceFiles, assemblies, sizing factors, run provenance, stageA, stageB or auditSchemaVersion live. Added a normative Top-level document subsection under Scope - four root properties plus a full nesting skeleton - placed as an unnumbered subsection to mirror SCHEMA.md and keep representations first, as agreed. Specifying the root exposed a duplication I had introduced: the model-versus-weather elevation comparison sat under each design day's location, though it is a property of the run. It now lives once at provenance.site, and designDay.location.elevation is just that day's own elevation. P1 - representations were declared but never checked against the payload, so a producer could declare ObservedHourly while emitting reconstructed series. New rule 18 binds each token to its evidence in both directions - ProcessedParametric to a parametric block, ObservedHourly to an available Observed series, DerivedHourly to an available Reconstructed series - and rejects an empty or unknown token list. This is what makes "declared, never inferred" enforceable rather than a matter of good intentions. P1 - `key` uniqueness was unstated while duplicate alignmentKey was handled explicitly. Since `key` is the Stage B join target, a duplicate would let one designDayKey resolve to several days, leaving a response attributable to no single set of conditions. New rule 17. P2 - Stage B required a non-null guid although METHODOLOGY.md states shared-gbXML translation may lose SAM GUIDs, so a legitimate shared-gbXML audit could not conform. guid is now string-or-null with null sorting last in any component that permits it, name is required and non-empty, and new rule 19 rejects a record that has neither. P2 - several newly introduced arrays had no declared sort key while §8 required one for every array, and `limitations` was only called "deterministically ordered". Added an Array ordering table giving a total key for all nine arrays, and pinned ordinal to StringComparer.Ordinal so output cannot depend on machine locale. Generated by Michal Dengusiak and CodeClaude Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs: separate engine echoes from engine results in the basis taxonomy Two more Codex findings, both real. P1 - the basis taxonomy had no valid state for an engine RESULT. Observed is authored input, EngineEchoed was defined as the engine's echo of what it accepted, Reconstructed is producer modelling, and Calculated is arithmetic - so a TSD design-day zone load had no truthful basis, and rule 7 additionally forbade EngineEchoed on a series. The Stage B hourly series that §7 promises was therefore unexpressible. The same hole affected the Stage B SCALARS, which I had implicitly treated as EngineEchoed although ZoneSizes design loads are results too. Fixed by splitting the engine-sourced basis in two: EngineEchoed is the engine restating an INPUT it accepted (the .eio design-day parameter echo), EngineReported is a RESULT the engine computed (a TSD zone load, a ZoneSizes sizing load). That distinction is the axis this audit turns on - an echo says what the engine was asked to simulate, a report says what it produced - and collapsing them would hide the difference between a design day being mistranslated and a design day being simulated differently. Stage A is largely echoes, Stage B is entirely reports, which is also why the two stages cannot share a status. Series basis is now Observed, Reconstructed or EngineReported, with a table fixing which applies per stage and source, and EngineEchoed still forbidden on a series. Rule 7 additionally requires every available Stage B value and series to be EngineReported. P2 - the Stage B compound sort key was called total but nothing forbade duplicates, so two records sharing (guid, name, designDayKey, loadType) would tie and serialization would fall back to producer enumeration order. New rule 21 requires uniqueness: one space, one design day and one load type yield exactly one record, so a duplicate is an extraction defect. Generated by Michal Dengusiak and CodeClaude Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs: allow DDY-shared alignment and enforce the parametric echo basis Three more Codex findings, all real. P1 - the alignment rule made the Ddy origin uncomparable. I had written that a null key is expected for `Ddy`, which forced every design day on an explicitly supported route to be reported one-sided, even though both producers can read the SAME DDY file and its sourceFiles hash proves the bytes are identical. The key now derives from whichever ancestor the two routes share, declared by a new `alignmentKeySource` of SamModel, DdyFile or None, and a null key is reserved for a genuinely untraceable day (`origin: Unknown`). Pairing gained two safety conditions the DDY case needs: the two days' `alignmentKeySource` values must agree, so a SamModel key and a DdyFile key are never interchangeable; and a DdyFile pairing additionally requires the two documents' Ddy sourceFiles hashes to be equal, because ASHRAE station names repeat across weather files and identically named days from different DDY files must not pair. Rule 10 now also cross-checks `alignmentKeySource` against `origin` and against the key's own nullability. P1 - the parametric echo basis was normative prose with no rule behind it. Rule 7's first half constrained only series and its second half only Stage B, so a producer could label a Stage A parametric field Observed, Calculated or even EngineReported and pass validation - defeating the input-echo distinction the contract had just been restructured around. Rule 7 is now three clauses, the third requiring every available parametric value, token and flag to be EngineEchoed. P2 - Stage B uniqueness used the sort key rather than space identity, so two records with the same GUID, design day and load type but different names both passed while double-counting one space. Rule 21 now keys on the EFFECTIVE identity: (guid, designDayKey, loadType) when a GUID exists, falling back to name only when it does not, matching the GUID-first identity rule. Generated by Michal Dengusiak and CodeClaude Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs: require a provably shared ancestor before pairing design days Three more Codex findings, all real. P1 - the embedded-model pairing had no equivalent of the DDY hash guard I had just added, so two audits of DIFFERENT SAM models that happen to reuse a conventional design-day name would produce the same alignmentKey, both declare SamModel, and pair. Design-day names are conventional labels - London_TRY ANN HTG 100% CONDS DB is a CIBSE name, not an identifier - so this was likely rather than theoretical. Added provenance.model carrying sourceModelName, sourceModelGuid, canonicalModelHash and canonicalizationVersion, computed exactly as for benchmark-<engine>.json, and made every pairing conditional on the ancestor being provably shared: SamModel requires equal canonicalModelHash AND equal canonicalizationVersion (hashes from different canonicalizers are not comparable evidence, per SCHEMA.md), DdyFile requires equal Ddy file hash. A name match alone now never authorises a pair. P2 - Stage B `conditioned` and `setpointState` were bare boolean and string, so they could carry neither `available` nor the required EngineReported basis, and null encoded absence outside the availability discipline. They are now AuditFlag and AuditToken; AuditToken's definition widened to cover a verbatim engine label as well as an enumeration token, which avoids a fourth shape for the same comparison semantics. §4 now states the general rule that no audit field is ever a bare JSON scalar, and new rule 22 enforces it. P2 - one-series-per-quantity was assumed by the "already total" series sort key but never required, so two competing profiles for one quantity would tie and leave byte order dependent on producer enumeration. New rule 21. Generated by Michal Dengusiak and CodeClaude Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs: make collapse loss genuinely one-sided Four more Codex findings, all real; the first corrected a substantive claim. P1 - collapse loss was not one-sided as advertised. ObservedHourly was defined as TAS-only from TBD, so comparing it against EnergyPlus's echo folded any SAM -> gbXML -> TBD translation change into the result and could misattribute it to OpenStudio. Fixed without adding a fourth representation: ObservedHourly now declares `observedFrom`, either SamModel (the authored SAM DesignDay, before any engine route) or EngineStore (the engine's own store, carrying that route's translation). Both producers load the same SAM model, so EITHER can emit a SamModel profile - including the OpenStudio producer alongside its own ProcessedParametric and DerivedHourly, which makes collapse loss a within-document comparison needing no cross-document pairing at all. Collapse loss now requires the SamModel profile. Comparing a TBD profile against the EnergyPlus echo is a distinct third pairing - route translation - and the contract says so rather than letting it pass as collapse loss. New rule 21 enforces `observedFrom`. P2 - the humidity shape could not express two EnergyPlus condition types. Added `J/kg` for Enthalpy and a `humidityConditionScheduleName` token for schedule-based types, with an explicit statement that v1 preserves the echoed schedule identity but does NOT reconstruct schedule-based humidity: the humidity series is unavailable and reconstruction.limitations must record it. Recording the identity and declaring the reconstruction unavailable beats fitting a profile the engine did not use. P2 - the DDY hash precondition was per-document while an audit may draw days from several DDY files, so the comparator could not tell which hash had to match. Each day now names its contributing file in `alignmentKeySourceFile` and only that file's hash is compared; rule 10 validates the reference resolves to a Ddy-role entry. P2 - ProcessedParametric could be declared over an all-unavailable block, advertising evidence the engine never echoed, unlike the hourly representations which require an available series. Rule 18 now requires at least one available value, token or flag. Generated by Michal Dengusiak and CodeClaude Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Generated by Michal Dengusiak and CodeClaude Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…t files from upstream contribution Dated report outputs (comparison/benchmark JSON, CSV, MD) are reproducible run results, not referenced by source or tests. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Benchmark implementation plan: drop the model-allocation table, the paste-ready first-implementation prompt and the mandatory-git/session-continuity block (replaced by one sentence on branch hygiene); neutralise stakeholder wording. Co-Authored-By: Claude Sonnet 5.5 <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.
Summary
Adds the cross-engine benchmark suite (SAM.Analytical.Benchmark, Compare, tests and fixtures), benchmark methodology/schema/tolerance documentation, design-day audit contract draft and CI workflow.
Validation
Test projects and fixtures included; built and run on the SAM-BIM fork. Generated run reports were removed from the branch.
🤖 Generated with Claude Code