Repository navigation
Q2 2026 — System.Text.Json migration + quarterly work - #7
Merged
Merged
Conversation
SAM-BIM core migrated from Newtonsoft.Json to System.Text.Json.Nodes. - Replace each csproj's Newtonsoft package reference with System.Text.Json 10.0.8 - ShortDateTime: FromJObject/ToJObject -> FromJsonObject/ToJsonObject; JObject -> JsonObject; Value<int>(key) -> indexer + GetValue<int>() Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
During the SAM-BIM Newtonsoft.Json -> System.Text.Json migration the quarterly sow/2026-Q2 branch carries the binary-breaking change. CI needs to consume the migrated SAM (and any sibling dep) from sow/2026-Q2, not from master, until the quarter-end merge. - Add "sow/2026-Q2" to push/pull_request branch triggers - For each dep clone, ls-remote sow/2026-Q2 and prefer it when present; fall back to default branch (e.g. master) otherwise. After the quarter merges back to master, this fallback naturally restores prior behaviour. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Refines the previous workflow patch so CI on the migration PR can succeed before anything has been merged. Each dep repo is cloned from: 1. github.head_ref (feature/remove-newtonsoft on these PRs) - has the migration right now on every dep 2. sow/2026-Q2 - source of truth after these PRs merge 3. default branch - source of truth after quarter-end merge Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The SPDX-check workflow requires the top 6 lines of every changed .cs file to contain both: // SPDX-License-Identifier: LGPL-3.0-or-later // Copyright (c) 2020-2026 Michal Dengusiak & Jakub Ziolkowski and contributors (with an en-dash between the years). Migration commits modified existing files without preserving the header. Prepend the two lines now. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Replaces the older spdx-check.yml (literal en-dash grep, top 6 lines, PR-only trigger) with the modern template shipped on SAM core: - grep -qE with [-–] char class — accepts both hyphen-minus and en-dash - 20-line lookback (was 6) - BOM and CR stripping - mapfile + diff-filter for cleaner change detection - Adds workflow_dispatch trigger Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Remove Newtonsoft.Json — migrate to System.Text.Json.Nodes
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a96d6bedc2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+60
to
+63
| $has = (git ls-remote --heads "https://github.com/$org/$r.git" $cand 2>$null | Out-String).Trim() | ||
| if ($has) { | ||
| Write-Host "Cloning $org/$r @ $cand" | ||
| git clone --depth 1 --branch $cand "https://github.com/$org/$r.git" $r |
There was a problem hiding this comment.
git ls-remote --heads ... $cand treats $cand as a pattern, not an exact ref name, so it can report a match for branches like team/$cand; in that case the subsequent git clone --branch $cand fails because $cand itself does not exist. This can break CI for PR branch names that are suffixes of another branch in a dependency repo, even though the fallback logic is intended to avoid clone failures.
Useful? React with 👍 / 👎.
Aligns the repo with the SAM-BIM workspace-wide pin to the LTS-aligned System.Text.Json 8.0.5, replacing the previously pinned 10.0.8 preview.
Pin System.Text.Json to 8.0.5
* fix: pin AssemblyVersion/FileVersion to 1.0.0.0 (drop 1.0.* wildcard) Eliminates CS1607 warnings and restores deterministic builds. Aligns with the workspace's existing fixed-version files. Scope: AssemblyInfo.cs + .csproj version attributes only. The // commented-out template example is left intact for documentation. * fix: add SPDX + copyright header to modified AssemblyInfo.cs and refresh .csproj <Copyright> Satisfies the spdx-check workflow which requires every changed .cs file to declare the LGPL-3.0-or-later SPDX identifier and the 2020-2026 attribution in its first 20 lines. Also updates the stale '<Copyright>Copyright (c) 2020</Copyright>' line in .csproj files to match the SPDX header style.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
* build: Directory.Build.props for centralised SAMVersion stamping Mirrors SAM-BIM/SAM#7. Stage 2 of the AssemblyVersion versioning migration. * fix: replace literal u{2013} escape with actual en-dash in SPDX header PowerShell 5.1 doesn't support backtick-u escape sequences; the apply-stage2.ps1 script leaked them as literal text in 'Copyright (c) 2020u{2013}2026'. * fix: relax SAMVersion.g.cs Target condition to != 'true' Codex P1 on SAM_LadybugTools#4: classic (non-SDK) csprojs don't set GenerateAssemblyInfo at all, so the previous '== false' condition skipped them. Switching to '!= true' catches both GenerateAssemblyInfo=false (SDK projects with legacy AssemblyInfo.cs) AND empty (classic projects). * ci: Codex P2 fixes — four-part SAMVersion, prefer head_ref for sow PRs - Append .0 so SAMVersion is 4-part (AssemblyFileVersion expects 4 parts). - Prefer github.head_ref when it matches sow/yyyy-Qx (release-promotion PRs). - Mirrors SAM sow/2026-Q2 6d87d98e.
Mirrors SAM sow/2026-Q2. Adds AssemblyInformationalVersion attribute generation to Path D's SAMVersion.g.cs Target — covers Grasshopper / Tas-bridge assemblies that have GenerateAssemblyInfo=false, so they also get the CI commit SHA stamp in their ProductVersion field. SDK projects pick up InformationalVersion via PropertyGroup auto-gen as before. Local dev builds unchanged.
Mirrors SAM sow/2026-Q2. Adds AssemblyInformationalVersion attribute generation to Path D's SAMVersion.g.cs Target — covers Grasshopper / Tas-bridge assemblies that have GenerateAssemblyInfo=false, so they also get the CI commit SHA stamp in their ProductVersion field. SDK projects pick up InformationalVersion via PropertyGroup auto-gen as before. Local dev builds unchanged.
ZiolkowskiJakub
approved these changes
May 27, 2026
michaldengusiak
added a commit
that referenced
this pull request
Oct 6, 2026
* Add SAM simulation engine roadmap
* refactor(gh): modernise legacy Grasshopper components (#6)
Convert OpenStudioCreateDesignDaysBySQL, OpenStudioCreateSpaceSimulationResultsBySQL,
and SAMAnalyticalAddResultsBySQL from GH_SAMComponent + imperative
RegisterInputParams/RegisterOutputParams to GH_SAMVariableOutputParameterComponent
with declarative GH_SAMParam[] Inputs/Outputs and name-based
Params.IndexOfInputParam/IndexOfOutputParam lookups, following the pattern
established by SAM PR #40 and SAM_Tas PR #16.
ComponentGuid, parameter order, names, nicknames, descriptions, access,
optional state, and persistent defaults preserved exactly (verified via a
generic reflection-based contract-capture harness: 0 unexplained differences
between baseline and migrated builds). LatestComponentVersion bumped by one
patch each.
Note: this repo's SAM.Analytical.OpenStudio project requires building with
-p:Platform=x64 (the OpenStudio NuGet package does not support AnyCPU) and
depends on SAM_SQLite's build output (SAM.Core.SQLite.dll) as a sibling
reference — neither is affected by this change, both pre-existing.
Generated by Michal Dengusiak and CodeClaude
* Implement native SAM analytical model to OpenStudio conversion (#7)
* docs: add SAM to OpenStudio MVP implementation plan
Optimised execution plan for the AnalyticalModel -> OpenStudio Model MVP
(Roadmap Phase 1 + adapter structure of Phase 2): single-run implementation
by Claude Fable 5 (Max) across milestones M0-M8 with per-gate commits and a
resumable status file, independent Codex GPT-5.6 Sol (Max) review, P0/P1 fix
round, and final validation incl. a mandatory Rhino 8 smoke test before the
PR into sow/2026-Q3.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* docs: audit OpenStudio toolchain and existing conversion code
M0 toolchain gate:
- adopt OpenStudio SDK 3.10.0 (from 3.8.0) matching the installed CLI
3.10.0+86d7e215a1 at C:\Program Files\ladybug_tools\openstudio
- add Query.OpenStudioCliPath with documented discovery order
(explicit -> PATH -> direct installs -> ladybug_tools)
- add net8.0 x64 NUnit test project with two passing smoke tests:
SDK create/save/reload round-trip outside Rhino, and CLI opening a
generated OSM via OSVersion::VersionTranslator
- pin Boston Logan TMYx 2004-2018 EPW/DDY/STAT weather fixture with
SHA-256 checksums and provenance
- remove empty SAM.Geometry.OpenStudio placeholder Test.cs
- record keep/reuse audit of the existing SQL results-import code and
the resumable run status file
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat: add OpenStudio conversion contracts and diagnostics
M1: OpenStudioDiagnostic(+Severity,+Codes), OpenStudioConversionOptions,
OpenStudioRunOptions/RunResult, OpenStudioObjectReference/Map with
duplicate prevention, OpenStudioConversionContext (caches, diagnostics,
registration) and immutable OpenStudioConversionResult snapshot with
IsValid; deterministic naming (SanitizeName, OpenStudioName
SAM_<Type>_<Name>_<Guid8>) and SDK/EnergyPlus version queries.
14/14 tests green.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat: convert SAM geometry primitives to OpenStudio
M2: Point3D->Point3d, ordered point collections, Face3D external edge
-> cleaned Point3dVector; CleanVertices (consecutive/closing duplicates,
collinear and spike vertices, wrap-aware, iterative), Newell Normal,
Area, IsPlanar, IsClockwise and ValidatePolygon with SAM-OS-GEO-001/002
diagnostics; no triangulation, no repair of non-planar polygons;
tolerances parameterized. SAM.Core/SAM.Geometry references added to
SAM.Geometry.OpenStudio. 24/24 tests green.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat: convert spaces zones stories surfaces and apertures
M3: AnalyticalModel.ToOpenStudio orchestrator with three passes -
A) BuildingStories from MinElevationDictionary, one Space+ThermalZone
per SAM space with nearest-story assignment; B) space-facing panels
via AdjacencyCluster.UpdateNormals (external normals, cloned panels)
converted to Surfaces with validated/cleaned polygons, plus Apertures
to SubSurfaces (FixedWindow/Door/GlassDoor via pane MaterialType,
host-plane coplanarity check, winding aligned to panel normal);
C) boundary conditions from the PanelType table and explicit
SAM-topology adjacent-surface/subsurface pairing (never geometric
matching), Adiabatic + SAM-OS-ADJ-001 warning for unpaired internal
panels. Shade panels to a building ShadingSurfaceGroup. Source model
copied and preprocessed (OffsetAperturesOnEdge, ReplaceTransparent
Panels) exactly as SAM_LadybugTools. Two-box fixture: counts, BCs,
mutual pairing with opposite normals, out-of-space normals, areas
20 m2 / volumes 60 m3, window 2.8 m2. 31/31 tests green.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat: convert analytical materials and constructions
M4: verified SAM layer convention (inside->outside per SAM XML docs;
'following the TAS approach') and mapped forward construction =
reversed SAM list (EnergyPlus outside-first). Material converter:
OpaqueMaterial->StandardOpaqueMaterial, TransparentMaterial->
StandardGlazing (LadybugTools front/back parity, flagged for review),
GasMaterial->Gas (Air/Argon/Krypton/Xenon only), cached per
(Guid,thickness), layer thickness from ConstructionLayer with
DefaultThickness fallback warning, validated physical values, no
hidden defaults. Construction converter cached per (Guid,direction),
pane variant for ApertureConstruction, setLayers failure surfaced.
Orchestrator Pass D: forward on primary/single-sided surfaces,
reverse on paired side, shared ConstructionAirBoundary for Air
panels, pane constructions on subsurfaces. Material mapping doc;
fixture upgraded to real 3-layer wall + glass/air/glass window and
material library; SPDX headers aligned to LGPL-3.0-or-later repo
convention. 38/38 tests green incl. layer order proven on both sides
of the internal panel.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat: convert profiles internal conditions and space loads
M5: Profile -> ScheduleFixedInterval with 8760 hourly values (annual
pass-through, weekly sub-profile cycling with LadybugTools parity,
divide/average day normalization, cyclic-tiling fallback with warning),
typed ScheduleTypeLimits (Fractional/Temperature/ActivityLevel), cached
per profile Guid. InternalCondition -> SpaceType with PeopleDefinition/
People (density, activity-level constant schedule, radiant 0.3 default,
sensible-heat fraction), Lights (radiant 0.32 / visible 0.25 defaults),
ElectricEquipment (radiant 0), SpaceInfiltrationDesignFlowRate per
sun-exposed exterior area (LadybugTools formula parity) and per-space
DesignSpecificationOutdoorAir from SpaceParameter.OutsideSupplyAirFlow.
Missing profiles raise SAM-OS-SCH-001 errors, never AlwaysOn.
SpaceType dedup is by sanitized NAME - verified against SAM sources:
Space.InternalCondition setter clones with a new Guid per space.
Profile values read via Min..Max indexer (Count is the index span).
Fixture: office IC + six profiles + space Area/Volume/OA parameters;
external walls now PanelType.WallExternal (ExposedToSun semantics).
44/44 tests green.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat: assign thermostats ideal loads and add simulation runner
M6 - first end-to-end EnergyPlus simulation green. Central
Query.IsConditioned (LadybugTools name convention); dual-setpoint
thermostats from Heating/Cooling profiles with pointwise
heating<=cooling validation (SAM-OS-HVAC-001 on violation or missing
setpoints); one ZoneHVACIdealLoadsAirSystem per conditioned zone
(documented EnergyPlus defaults); EPW validation + WeatherFile + Site
population; MVP simulation settings (annual run period, 6 timesteps/h,
weather-run only, 6 hourly output variables); OpenStudioSimulationRunner:
OSM save, minimal OSW, CLI discovery with ladybug_tools fallback,
timeout+kill, eplusout.err severe/fatal parsing, SQL discovery and
annual kWh extraction per report key; full-pipeline
ToOpenStudio(analyticalModel, epwPath, outputDirectory) overload;
OpenStudioLoadSummary with finite/non-negative checks.
E2E gate met on the one-zone Boston fixture: exit 0, no fatal/severe,
heating 2621.7 kWh and cooling 1745.0 kWh (both > 0), zone-level
result count equals conditioned-zone count. 48/48 tests green.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* test: add SAM OpenStudio MVP regression fixtures
M7: fixture suite completing plan section 12 - two stacked boxes
(story assignment 0m/3m and RoofCeiling/Floor flip on the shared
internal floor), conditioned/unconditioned pair (HVAC only on the
conditioned zone, gains retained on both), irregular planar room
(vertex cleaning warning, area preserved), degenerate-panel failure
policy (SAM geometry kernel NRE isolated to SAM-OS-GEO-001 space
error + per-panel no-silent-drop warnings, zero surfaces, result
invalid, no repair), repeated-conversion source-mutation guard,
deterministic-name guard, disposal loop, and a second E2E EnergyPlus
run (two zones, both reporting heating loads through the matched
internal wall). Converter hardening: exception isolation around
SAM geometry-kernel calls (story elevations per panel with
floor-group filter, space shell, panel conversion, exposed-area);
no-silent-drop reporting for panels that produce no surface.
Restored externally wiped sibling prebuilts (SAM build at 487bd679,
SAM_SQLite) by rebuilding artifacts only. 56/56 tests green.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat: expose OpenStudio conversion in Grasshopper and document workflow
M8: thin Grasshopper components following the repo's
GH_SAMVariableOutputParameterComponent pattern - SAMAnalytical.ToOpenStudio
(AnalyticalModel + EPW + output folder, _run-gated, outputs OSM/OSW/SQL
paths, annual heating/cooling kWh, diagnostics, success) and
OpenStudio.RunModel (existing OSM/OSW through the CLI); no conversion
rules in components. Runner gains a save-only mode (run:false still
saves OSM/OSW) and a standalone Run(path, epw, outDir) API for existing
models; existing SQL-results components reused for result import.
Usage doc (prerequisites, C# API, components, documented limitations)
and the MVP acceptance checklist - all 11 plan definition-of-success
criteria met. 56/56 tests green; full solution builds 0 errors.
In-Rhino smoke test remains the mandatory human Stage-5 gate.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix: align weekly profile day-of-week with the run calendar (review P1-01)
Day-composed (weekly) SAM profiles were tiled Monday-first regardless of the run calendar, so with an EPW whose 1 Jan is not a Monday (Boston TMYx: Sunday) every weekday/weekend pattern landed on the wrong calendar day. The full-pipeline overload now derives the 1-Jan day of week from the EPW (EpwFile.startDayOfWeek), OpenStudioConversionOptions.FirstDayOfWeek allows an explicit override, and the weekly cycle is rotated onto the calendar (sub-profile 0 = Monday per the LadybugTools ScheduleRuleset convention). Weather-free conversions keep the Monday-first default. Regression tests: weekly profile with Monday=1/weekend=0 aligned through the EPW pipeline, Monday-first default, and the explicit override.
* fix: stop silently squashing leap-year and multi-day profiles (review P1-02)
Profiles with more than 8760 values (8784 leap-year) or multi-day flat sequences (e.g. 168-hour weeks) were block-averaged into a single synthetic average day with no diagnostic. Expansion now follows SAM's native semantics (Profile.GetYearlyValues / the wrapping indexer): 8760 as-is; longer profiles truncated to the first 8760 hours with a SAM-OS-SCH-001 warning; flat profiles tiled hour-for-hour at their own period; gaps raise an error instead of embedding NaN. Regression tests: leap-year truncation + warning, weekly flat profile tiled hour-for-hour, daily profile unchanged, gappy profile error.
* fix: deduplicate SpaceTypes by name plus content hash (review P1-03)
SpaceType deduplication keyed on the sanitized internal-condition name only, silently merging conditions that share a name but differ in gains or profile references (SAM clones conditions with new Guids per space, so neither Guid nor name alone identifies them). The dedup key is now name + deterministic content hash over the condition's full parameter content with Guids excluded: identical clones still share one SpaceType; same-named conditions with different content get distinct SpaceTypes with correct per-space loads. Regression tests: same name/different gains, same name/different profiles, identical clones dedup + hash determinism.
* fix: map glazing optical sides to EnergyPlus front/back correctly (review P1-04)
SAM Internal* reflectances/emissivity were written to the EnergyPlus glazing Front side and External* to Back. EnergyPlus defines Front as the side of the layer opposite the zone (exterior-facing for exterior windows), so SAM External* must map to Front and Internal* to Back. The previous assignment mirrored SAM_LadybugTools EnergyWindowMaterialGlazing.cs, which carries the same inversion; the deviation is documented in the material mapping doc. Symmetric glass (including the fixture) is unaffected, which is why no test caught it. Regression test: asymmetric coated glass verifies all six front/back assignments.
* fix: make the CLI timeout real and deadlock-safe (review P1-05)
ExecuteCli read StandardOutput.ReadToEnd() then StandardError.ReadToEnd() before WaitForExit(timeout): the timeout never fired while the process ran (proven: 120.6 s block with TimeoutSeconds=5, then exit 0 reported), and a child filling the stderr pipe while stdout stayed open could deadlock parent and child permanently. Now: asynchronous reads via BeginOutputReadLine/BeginErrorReadLine, WaitForExit(timeout) that actually applies, and process-tree termination via a kill-on-close Job Object (TerminateJobObject on timeout, root Kill as fallback, handle close as backstop). Err-file parsing after a kill is best-effort (the dying process can hold the file locked). Regression test: real CLI with TimeoutSeconds=1 fails with exit -1 + TIMEOUT diagnostic in bounded time, no orphaned openstudio/energyplus processes.
* fix: reject self-intersecting polygons (review P2-01)
ValidatePolygon checked duplicates, collinearity, planarity and minimum area but never self-intersection, so a planar self-intersecting polygon with non-zero signed area (star) was accepted silently and became an invalid EnergyPlus surface. Adds Query.IsSelfIntersecting (dominant-axis 2D projection, O(n^2) proper-crossing test with tolerance-aware touch detection) wired into ValidatePolygon as a SAM-OS-GEO-001 error. Regression tests: star and bowtie rejected, concave L-shape and the full fixture set unaffected.
* fix: sanitize quotes out of OpenStudio names (review P2-02)
SanitizeName stripped IDF-breaking characters but kept single and double quotes; those flowed into EnergyPlus report keys and broke the SQL extraction literal (WHERE rdd.KeyValue = '...'), silently dropping the zone's results. Quotes now collapse to underscores like the other invalid characters. Regression tests: SanitizeName removes both quote kinds; a space named O'Brien converts with a valid deterministic name.
* docs: complete independent review with fixes and final validation
Review doc finalised: 0 P0, 5 P1, 2 P2 findings all resolved with regression tests (commits 4d28513..8c3367f), Stage-D validation (73/73 tests, three post-fix EnergyPlus runs), environmental note on the shared-machine Rhino lock. Status and usage docs updated for the review fixes: weekly-profile calendar alignment and FirstDayOfWeek option, leap-year truncation policy, SpaceType content-hash names, real CLI timeout with process-tree kill, self-intersection rejection, quote sanitization, glazing front/back correction.
* fix: distinguish opaque air gaps from window gas layers (review P1-06)
The real Rhino 8 smoke-test model failed fatally in EnergyPlus: SAM GasMaterial in opaque constructions was converted to OS:WindowMaterial:Gas, which reports R = 0.000 to InitConductionTransferFunctions (AR90UP 50 mm cavities in wall SIM_EXT_SLD and roof SIM_EXT_SLD_Roof). Material conversion now takes an explicit OpenStudioMaterialUsage context: opaque constructions map SAM GasMaterial to OpenStudio.AirGap with R = 1/h from GasMaterialParameter.HeatTransferCoefficient [W/m2K] (SAM's authoritative cavity conductance, written by Query.UpdateHeatTransferCoefficients; opaque-cavity model per BS EN ISO 6946 via Query.AirspaceThermalResistance), rejecting missing/invalid conductance and resistances below the EnergyPlus 0.001 m2K/W minimum with SAM-OS-MAT-001 before CLI execution; ApertureConstruction panes keep OpenStudio.Gas with gas type and thickness. Material cache keys include the usage (Guid:OpaqueAirGap:R / Guid:WindowGas:thickness) so one SAM GasMaterial used in both contexts yields two distinct objects; construction-level family validation rejects Gas in opaque sets, AirGap in pane sets and mixed families, with no silent layer omission. 9 new tests including an end-to-end opaque-air-gap EnergyPlus run (exit 0, 0 fatal/severe). Suite: 82/82. Mapping, review, status and usage docs updated.
* test: validate OpenStudio MVP with real Rhino model
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* docs: audit complete SAM analytical translation coverage
Full audit of the SAM.Analytical.AnalyticalModel data surface against the
native OpenStudio translation: 279 reviewed entries classified as Native /
Derived / Approximated / Unsupported / Deferred / NA, each with unit,
OpenStudio representation, conversion formula and diagnostic policy.
- docs/SAM_OPENSTUDIO_ANALYTICAL_COVERAGE.md: human-readable audit; tables
generated from the manifest so both stay in lockstep.
- tests/resources/openstudio-analytical-coverage.json: machine-readable
manifest consumed by the C7 completeness tests (tests never parse
Markdown). coveredEnums lists every SAM parameter/type enum whose live
membership must stay fully classified.
- docs/openstudio-analytical-completeness-status.md: running programme
status and gate log.
Enum membership extracted mechanically from SAM core source. Agreed
reclassifications recorded: dividers N/A (no SAM source data), blinds and
opening properties unsupported-with-diagnostic, STAT parsing deferred,
subhourly profiles N/A (hour-indexed stores), SAM hourly design days
approximated with DDY import as the deterministic path.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* refactor: harden OpenStudio analytical adapter
* feat: complete internal condition and schedule translation
* feat: complete fenestration and construction translation
* feat: complete OpenStudio weather and simulation settings
* feat: expand OpenStudio results translation
* feat: add cancellable OpenStudio simulation execution
* test: validate complete SAM analytical translation
* fix: deploy System.Data.SQLite assets so Ideal Loads results can be read
The Grasshopper plugin reported "SAM-OS-RUN-001 - Ideal Loads results
could not be extracted" on every run, even though EnergyPlus completed
successfully and eplusout.sql held 8760 rows per Ideal Loads variable.
SAM.Analytical.OpenStudio and the .gha are class libraries, so
CopyLocalLockFileAssemblies defaults to false and the SDK never copies
PackageReference runtime assets to the output folder. Neither
System.Data.SQLite.dll nor SQLite.Interop.dll reached %APPDATA%\SAM, so
the first touch of a SQLite type threw FileNotFoundException, and a
blanket catch rewrote it as a message that read like missing data.
Copy just the two SQLite assets from the resolved package, preserving the
runtimes\win-x64\native layout that both System.Data.SQLite's own loader
and SAM's AssemblyResolver probe. Deliberately not
CopyLocalLockFileAssemblies=true, which would also push System.Text.Json
and 8 more facades into a folder shared by every SAM plugin and loaded
into Rhino's default AssemblyLoadContext. No RuntimeIdentifier is needed:
RuntimeTargetsCopyLocalItems already carries the win-x64 asset on a
RID-less build.
Also surface what actually failed instead of swallowing it: the
diagnostic now carries the exception type, the full inner-exception
chain, the SQL path, where System.Data.SQLite loaded from, the plugin and
base directories and the process bitness - enough to tell a missing
managed DLL, a missing native DLL, a wrong architecture and a real query
failure apart. Two previously silent nulls now report, and cancelling an
async component no longer throws on Task<T>.Result.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix: wind panel surfaces to the panel outward normal
A model with windows made EnergyPlus terminate before simulating: 12
severe "checkSubSurfAzTiltNorm: Outward facing angle of subsurface
differs more than 90.0 degrees from base surface".
Panel conversion built the OpenStudio Surface straight from the Face3D
external-edge vertex order, while aperture conversion explicitly re-wound
its SubSurface to panel.Normal. panel.Normal is the PLANE normal, and
AdjacencyCluster.UpdateNormals - which the converter calls - flips that
plane to point out of the space without rewinding the stored boundary.
Wherever the two disagreed, host and window came out exactly 180 degrees
apart.
Measured on the reported model's own generated IDF: all 6 base surfaces
faced into the space (floor +Z, roof -Z, every wall toward the centroid)
and all 12 subsurfaces sat at 180.0 degrees to their host. The same
inversion was already corrupting window-free models silently - EnergyPlus
reported "Floor is upside down", "Roof/Ceiling is upside down" and
"Indicated Zone Volume <= 0.0", so anything derived from zone volume was
computed against a degenerate volume rather than failing loudly.
Align the base polygon to panel.Normal, exactly as the aperture already
does, so both sides share one winding authority. Verified on a freshly
generated IDF: roof +Z, floor -Z, all walls outward, subsurface 0.0
degrees from its host, and the upside-down and zone-volume warnings gone.
The added tests assert the invariant but do NOT reproduce the original
divergence - a fixture cannot, because Polygon3D re-derives its plane
from the points given, so reversing vertices flips panel.Normal with
them. That limitation is documented in the test file.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* docs: record independent completeness review findings (stage C)
Independent reproduction of the C0-C7 baseline (build, 146/146 suite, five
EnergyPlus scenarios through a standalone driver), coverage-system audit and
per-milestone review. Findings: three P1 (declared-but-never-emitted coverage
diagnostics; default DDY filter selects no cooling design day; leap-year
Feb 29 hour-of-year collapse), six P2, eight P3. Fixes follow one commit per
finding.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix: emit declared coverage diagnostics for unconverted parameters (review P1-01)
Eighteen coverage-manifest rows declared structured diagnostics that no
converter ever emitted (false completeness). The declared SAM-OS-IC-001 /
SAM-OS-HVAC-001 / SAM-OS-CON-002 / SAM-OS-MAT-002 information diagnostics
now fire at the natural conversion sites:
- InternalCondition: occupancy/equipment view coefficients, lighting
control function, and grouped emitter/exhaust deferrals naming every
present parameter (SAM-OS-HVAC-001 summary widened accordingly, P3-05)
- Aperture: aperture-construction internal-shadow flag
- Construction: construction internal-shadow flag, once per construction
- Panel: panel-level feature shade, once per panel
- Material: vapour diffusion factor (any material kind, once per
material) and divergent opaque internal-side optics (single-sided
approximation), via the new OpenStudioConversionContext.RegisterOnce
No conversion behaviour changes otherwise; clean fixtures stay silent.
Regression tests: C7/DeclaredDiagnosticsTests (8 tests, one per emission
site, once-per-object enforcement and matching-optics silence).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix: select the ASHRAE heating/cooling design-day pair from DDY imports (review P1-02)
The default DDY filter matched any name containing "99.6%" or "0.4%":
ASHRAE files write the cooling day "Ann Clg .4% Condns DB=>MWB" with no
leading zero, so no cooling design day was ever imported, while the
humidification ("Ann Hum_n 99.6%") and wind ("Ann Htg Wind 99.6%") days
leaked in. Cooling sizing was silently computed from heating-condition
days.
Selection is now explicit and case-insensitive: the annual heating
99.6% dry-bulb day ("Ann Htg 99.6% Condns DB", wind day excluded by
adjacency) plus the annual cooling .4%/0.4% DB=>MWB (or MCWB) day. A
missing side raises a per-side warning; when neither matches the import
falls back to all days with the existing warning. ImportAllDesignDays
is unchanged.
Regression tests: the C4 import test now names the exact pair and
rejects Hum_n/Wind days; a heating-only DDY imports one day and warns
about the missing cooling side; the end-to-end sizing simulation
asserts (via SQL EnvironmentPeriods) exactly one heating and one
cooling sizing environment with annual totals identical to baseline.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix: compute SQL hour-of-year against the run calendar in leap years (review P1-03)
ReadHourlyValueSeries derived hour-of-year with a fixed non-leap
reference year (2023): in a leap-year run (IsLeapYear + leap EPW)
EnergyPlus reports Feb 29 rows, the day-clamp mapped them onto Feb 28 -
producing duplicate hour indices that CoincidentPeak summed into the
same 24 buckets (building coincident peak inflated up to 2x) - and
every post-February peak hour landed one day early.
The reader now detects Feb 29 in the rows it consumes (restricted to
the annual weather environment when one exists) and selects a leap
reference year (2024) for the day-of-year computation; the defensive
clamp remains for genuinely invalid dates. Non-leap indexing is
unchanged.
Regression tests (C5/LeapYearIndexingTests, synthetic EnergyPlus-schema
SQLite through the internal reader, now InternalsVisibleTo-exposed):
- leap dataset: 50 distinct indices, Feb 28 @1392, Feb 29 @1416,
Mar 1 @1440, Dec 31 @8783 - no duplicate hour keys;
- non-leap dataset: Mar 1 @1416, Dec 31 @8759 (original behaviour).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix: enforce stale-manifest-id detection and remove two stale rows (review P2-01)
The coverage document promised the completeness test fails when a
manifest entry references an enum member that no longer exists, but
only the live-member -> manifest direction was enforced: renames or
removals in SAM would leave stale coverage rows behind silently.
Manifest_HasNoStaleIds now checks the reverse direction - every
manifest id whose prefix belongs to a covered enum must name a live
member. On its first run it caught two REAL stale rows the C0 audit
had transcribed from commented-out code:
AnalyticalMaterialParameter.TypeName and .Description do not exist in
the live SAM.Analytical.MaterialParameter enum. Both rows are removed
from the JSON manifest and the Markdown document (byte-level edit
preserving the pre-P2-06 encoding); coverage totals corrected to 277
entries / NA 56 / C0 62 in the coverage and status documents.
Failure reproduction: an injected ghost id (and the two real stale
ids) fail the new check; the corrected manifest passes.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix: warn when a non-hourly output frequency mis-scales extracted peaks (review P2-02)
OutputVariableFrequency is honoured on the Output:Variable requests,
but the result extractor converts per-row energies with a fixed 3600 s
interval and assumes 8760/8784 hour indexing: "Timestep" under-reports
peaks, "Daily" inflates them, series-length assumptions break. Annual
sums are frequency-independent and stay correct.
Mitigation on this branch: a conversion-time warning under the new
SAM-OS-RUN-003 (ResultExtractionLimitation) code naming the requested
frequency and stating that peaks, peak hours and hourly series assume
hourly reporting. The request itself is unchanged and the default
hourly path stays silent. Frequency-aware extraction remains follow-up
work (review document, section 5).
Regression test: C4 NonHourlyOutputFrequency_WarnsThatPeaksAssumeHourly
(warning fires once for "Timestep", frequency still honoured, hourly
silent).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix: validate the SAM Location site override before writing OS:Site (review P2-03)
The SAM Location override copied Latitude/Longitude/Elevation into
OS:Site whenever a Location existed - a NaN or out-of-range value
reached the OSM and EnergyPlus unchecked.
The override now applies only when latitude is finite in [-90, 90] and
longitude is finite in [-180, 180]; otherwise the EPW site is kept and
a SAM-OS-RUN-002 warning names the rejected values. A non-finite
elevation keeps the EPW elevation while valid coordinates still
override, stated in the override message. The valid-Location
precedence and its information diagnostic are unchanged.
Regression tests (C4): NaN latitude and longitude 200 keep the EPW
site and warn; NaN elevation keeps the EPW elevation while overriding
coordinates.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix: cancel Grasshopper simulations on component removal and document close (review P2-05)
Deleting the component or closing the Grasshopper document left the
OpenStudio CLI / EnergyPlus run executing to completion (bounded only
by the timeout), and the completion continuation could call
ScheduleSolution on a disposed document.
- RemovedFromDocument cancels the running task; the runner's Job
Object kills the whole process tree (same path as cancel_ and the
timeout).
- DocumentContextChanged cancels on Close only - lock, unload and
document switches leave the run alive (a completed run harvests when
the document returns).
- The completion continuation re-resolves OnPingDocument() and never
schedules into a removed component or disposed document
(exception-guarded); the completed run is dropped by design.
Compile-verified against the full solution (Rhino closed); headless
tests cannot exercise the Grasshopper runtime - runtime confirmation
is step 5 of the human Rhino checklist in the review document.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix: repair encoding corruption in the coverage document and manifest (review P2-06)
docs/SAM_OPENSTUDIO_ANALYTICAL_COVERAGE.md carried UTF-8 text that had
been decoded as CP1252 and re-encoded: 3 arrows rendered as a-dagger
sequences, 12 em-dashes and 1 en-dash as euro-quote sequences. A full
byte-level scan located every corrupted run; all 16 are rewritten as
the intended UTF-8 code points (byte-exact replacement, no other
content touched).
tests/resources/openstudio-analytical-coverage.json carried one raw
CP1252 em-dash byte (0x97, invalid UTF-8) in the
Result.Space.GlazingExternalConduction note - now a proper UTF-8
em-dash (previously survived only because File.ReadAllText substitutes
U+FFFD).
Verification: both files decode under a strict exception-fallback
UTF-8 decoder, and the only remaining non-ASCII byte runs are the
intended arrow/dash code points.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* docs: record Stage L corrections and final validation evidence (stages L-M)
Review document: corrections table with the eight per-finding commit
SHAs and regression tests (section 8); final validation - clean full
solution build, 170/170 tests with 0 skipped, five post-fix EnergyPlus
scenario simulations and all special validations (section 9); READY
recommendation gated on the human Rhino checklist (section 12).
Status document: Stage L corrections section, gates-log rows for the
pre-Stage-C fixes and the review corrections, final summary updated to
170 tests / 277 manifest entries, and the review follow-ups (P2-04,
frequency-aware extraction) recorded.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix: add missing SPDX/copyright header to Create/Model.cs
CI's spdx check flagged this file (touched by C1, 0edf26f) as never
having received the required header. Added, byte-identical to the
header used by every other file in the project.
* feat: use analytical model weather and design days in OpenStudio conversion
Source-resolution precedence (never merged, always named in a diagnostic):
explicit EPW/DDY path > AnalyticalModel embedded WeatherData/design days >
documented fallback or blocking diagnostic.
- Weather: explicit EPW wins; embedded WeatherData with weather years is
exported through SAM.Weather's ToEPW API; without any EPW its metadata
(location, elevation/time zone, ground temperatures) still applies. A
missing source is a blocking error only when a run is requested;
conversion-only saves OSM/OSW with a warning.
- Design days: embedded SAM heating/cooling days are translated to
SizingPeriod:DesignDay (daily range from the 24 h profile, constant dew
point at the max-dry-bulb hour, mean wind/pressure, ASHRAEClearSky
clearness 0.0 heating / 1.0 cooling) with every approximation named in a
warning; explicit DDY (run option > conversion option) overrides and is
never merged with embedded days.
- Grasshopper SAMAnalytical.ToOpenStudio 1.2.0: optional _epwPath, new
optional ddyPath_ input appended after cancel_ (position pairing keeps
archived documents wired), real OpenStudioConversionOptions passed, and
the async signature keys on a deterministic fingerprint of the embedded
weather/design-day content (same model Guid + changed content => new run).
- Tests: +11 (EmbeddedWeatherDesignDaysTests) - 170 -> 181.
* fix: normalise EnergyPlus SQL timestamps when creating design days
EnergyPlus SQL Time fields denote the END of the reporting interval
(Hour 0-24, Minute 0-60, Year 0 on sizing environments) and were passed
raw into DateTime constructors: hour-0 sub-hourly rows produced hour -1
in TimeIndexDictionary and hour-24 rows threw in Query/DateTime — the
Grasshopper 'un-representable DateTime' solution exception.
- New central helper Core.OpenStudio.Query.TryGetDateTime: validates the
calendar date (Feb 29 in a non-leap calendar, month/day 0, hour > 24,
minute > 60 -> structured diagnostic, never a throw, never clamped) and
normalises by TimeSpan addition so day/month/year rollover is DateTime
arithmetic; IntervalHourOfYear maps an interval-end timestamp to the
0-based interval index by stepping one tick back into the interval.
- TimeIndexDictionary rewritten on the helper (inverted year-default
logic removed) with a diagnostics sink; Query/DateTime normalises
through it; SpaceSimulationResults peak indices use IntervalHourOfYear.
- Create.DesignDays: annual weather-run environments (EnvironmentType 3)
are not misread as design days; 24:00 rows stay with their design day;
missing series values no longer throw; days classified heating/cooling;
structured diagnostics out-parameter.
- OpenStudio.CreateDesignDaysBySQL 1.0.2 reports malformed rows as
runtime warnings instead of a solution exception.
- Tests: +18 (EnergyPlusTimestampTests, incl. the real 30 MB reproduction
fixture gated on SAM_OPENSTUDIO_TEST_SQL) - 181 -> 199.
* fix: add space and surface simulation results from SQL
SAMAnalytical.AddResultsBySQL returned no space/panel results: only
design-day ZoneSizes data was mapped (annual-only SQL yielded
area/volume-only results), Create.SurfaceSimulationResults discarded the
whole surface list when no space result carried a load time index, and
EnergyPlus names (SAM_<type>_<name>_<guid8>) were matched against full
32-character Guids so nothing related back to SAM objects.
- Modify.AddResults rewritten: annual family via the single authoritative
C5 reader (ExtractResultSet -> ToSAM_SpaceSimulationResults, new
IEnumerable<Space> overload); design-day ZoneSizes family kept when
sizing ran; one SurfaceSimulationResult per engine surface (internal
panel = two results on the same panel, never summed); zone/surface
names resolved by deterministic Guid suffix, never display name alone.
- Zero stays a valid result; only absence is diagnosed. Unmatched SQL
zones/surfaces and result-less SAM spaces/panels are reported as
structured diagnostics (Grasshopper warnings).
- Rerun dedup by (type, name, reference, load type); relations ensured.
- Component 1.0.5 clones the input cluster before attaching (non-mutating
convention); panelSimulationResults output name retained (SAM class
SurfaceSimulationResult, documented).
- Create/PanelSimulationResults: base surface list survives without
LoadTimeIndex.
- Tests: +9 (AddResultsBySqlTests incl. real-fixture gate) - 199 -> 208.
* test: end-to-end EnergyPlus gate for the fully-embedded route
Model with embedded WeatherData + embedded heating/cooling design days,
no explicit EPW or DDY: run succeeds, exactly one annual plus the two
embedded sizing environments are simulated, Create.DesignDays round-trips
them without a DateTime exception, and Modify.AddResults attaches the
annual space family plus per-surface results. Suite: 209 tests.
* fix: relate EnergyPlus subsurface results to the panel hosting the aperture
Every window row of a live eplusout.sql was reported as
"SQL surface result 'SAM_SUBSURFACE_<name>_<guid8>' matched no SAM panel;
left unattached" — 12 of 18 engine surfaces dropped on the validation model.
The EnergyPlus Surfaces table lists subsurfaces alongside base surfaces, and
Convert/ToOpenStudio/Aperture.cs names a SubSurface with the APERTURE Guid.
LookupPanel only compared that Guid suffix against panel Guids, which a window
can never equal. Apertures are not standalone AdjacencyCluster objects, so the
host panel is the only valid relation target; the result keeps its own SQL name
and SurfaceIndex reference, so window and opaque results stay distinguishable.
Verified against the real run (rawModel-WindowsWeatherDataDDY.sam +
simulation_v3/eplusout.sql): 12 diagnostics before, 0 after, all 18 engine
surfaces attached to the correct host panel with the correct areas.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* test: gate the exported EPW daylight-saving header
The embedded-WeatherData route exports an EPW through Weather.Convert.ToEPW,
whose HOLIDAYS/DAYLIGHT SAVINGS line carried an extra empty leading field. Every
run of the fully-embedded route therefore ended with an EnergyPlus Severe
(ProcessEPWHeader: Invalid Daylight Saving Period Start Date Field=NO). The
writer is fixed in SAM (SAM.Weather DaylightSavingsString); this asserts the
header EnergyPlus actually parses, field by field, at the consumer boundary.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix: record an absolute weather_file in the generated OSW
Codex review (PR #8, P2): with UseUniqueRunDirectory at its default the OSW is
written into a Guid subdirectory, but weather_file was recorded exactly as
supplied. OpenStudio resolves weather_file relative to the OSW, so a relative
EPW path that exists from the caller's working directory pointed nowhere and
the simulation ran without weather.
The path is now made absolute, matching what the standalone OSW writer in the
same file already did. An absent weather source (conversion-only, valid since
the embedded-weather work) still writes an empty entry rather than throwing.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix: key the SimpleGlazing cache on the resolved aperture performance
Codex review (PR #8, P2): the layerless-aperture fallback takes U / SHGC /
visible transmittance from the Aperture before the shared ApertureConstruction,
but the cache key held only the construction Guid and direction. Two apertures
sharing one construction with different aperture-level overrides collided - the
second window silently reused the first window's SimpleGlazing and simulated
the wrong glazing performance.
The resolved values are now part of the cache identity, so differing overrides
get their own SimpleGlazing while identical ones still share one. Overridden
sets also carry their performance in the OpenStudio name, keeping names
deterministic and the trailing-Guid resolution convention intact instead of
letting OpenStudio auto-rename a duplicate.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* docs: escape the Guid-suffix token in the AddResults doc comment
(SAM_<type>_<name>_<guid8>) was parsed as XML markup, so the project's
generated documentation file emitted four CS1570 warnings and silently
truncated the summary at the bad tag. The token is now escaped exactly as
Query/GuidSuffix.cs already writes it.
Repo-wide scan of every /// block under SAM_OpenStudio/ and Grasshopper/:
this was the only malformed one; the Query files added alongside it were
already correct.
Documentation only - no code change.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* chore: add the SPDX/copyright header to the eight files missing it
The spdx workflow checks every .cs file changed against the PR base and was
failing on PR #8 with eight "Missing SPDX line" entries - files that predate
the header convention and were touched by this branch:
Analytical.OpenStudio: Create/DesignDays, Create/PanelSimulationResults,
Create/SpaceSimulationResults, Modify/AddResults,
Query/GuidSuffix
Core.OpenStudio: Query/DateTime, Query/TimeIndexDictionary,
Query/TryGetDateTime
Header only - no code change. Existing byte-order marks and line endings are
preserved, and the copyright line matches the en dash the rest of the tree uses.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix: reference results by SAM Guid and resolve windows through their host surface
Second Rhino validation round: one window of twelve still reported "matched no
SAM panel". Its aperture Guid was not in the cluster the results were attached
to - apertures are re-created (trimmed, merged, re-hosted), so a Guid that
existed at conversion time need not survive to AddResultsBySQL, and matching a
subsurface purely by aperture Guid cannot be relied on.
EnergyPlus already records the answer: Surfaces.BaseSurfaceIndex points every
subsurface at the base surface it sits in, and THAT name carries the panel Guid.
Resolution is now panel Guid -> aperture Guid -> host base surface, so a window
can only fail to resolve if its own wall did too. The column is optional; older
or synthetic schemas simply fall back to the first two routes.
Requested in the same review: Reference now carries the matched SAM Space /
Panel Guid, uniformly across both space families and all surface results, so a
consumer holding only the result list can map back to the model without
re-parsing EnergyPlus names. Core.Result.Reference is immutable, so the result
is rebuilt through its JSON form - Guid, name, source, timestamp and every
parameter carry over. The engine identity it replaces is kept as parameters:
SurfaceIndex and HostSurfaceName on surfaces, ZoneIndex and ZoneName on spaces.
Two engine surfaces of one internal panel now share a Reference and are told
apart by SurfaceIndex; values are still never summed.
Verified against the live run (rawModel-WindowsWeatherDataDDY.sam +
simulation_v4/eplusout.sql, with window 6F64626E's aperture deliberately absent
from the model): 1 diagnostic before, 0 after, all 18 engine surfaces attached
to the correct host panel, every result referencing its SAM Guid.
Also documents why one conditioned space yields four SpaceSimulationResults -
the annual family (Load, LoadIndex, UnmetHours) and the design-day ZoneSizes
family (DesignLoad) are distinct measurements, deliberately not merged.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* chore: add the SPDX/copyright header to the four files touched in c009487
Same gap as 197eba3: four more pre-existing .cs files without the header
convention were touched (Convert/ToSAM/PanelSimulationResults,
Convert/ToSAM/SpaceSimulationResults, Enums/Parameter/PanelSimulationResultParameter,
Enums/Parameter/SpaceSimulationResultParameter). Header only - no code change;
existing BOM and CRLF line endings preserved.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* docs: explain result Reference mapping and the two space-result families
Both questions came out of the Rhino validation, and the answers only lived in
the internal mapping reference. They belong where a Grasshopper user looks, so
openstudio-mvp-usage.md now has a "Reading SAMAnalytical.AddResultsBySQL output"
section covering:
- Reference is the mapping key (SAM Space/Panel Guid, 32-hex "N" form); do not
parse the result Name, which differs per family. Where the EnergyPlus identity
went (ZoneIndex/ZoneName, SurfaceIndex/HostSurfaceName), what an unmatched
result looks like, and why an internal wall gives two results sharing one
Reference.
- Why one conditioned space returns four SpaceSimulationResults: the annual
family (Load, LoadIndex, UnmetHours) and the design-day ZoneSizes family
(DesignLoad, DesignDayName, PeakDate) are separate measurements, deliberately
not merged, and legitimately differ in magnitude - with the filter to split
them (presence of Load vs DesignLoad, then LoadType).
Documentation only - no code change.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* refactor: adopt the SAM core async base, TryGetDateTime and Variables convention
Applies ZiolkowskiJakub's review of PR #8, all three points:
- GH_SamAsyncComponent -> SAM.Core.Grasshopper.GH_SAMAsyncComponent. The local
base was written here only because no task-capable base existed in SAM; it now
does (SAM 3b16ca4f), so the copy is deleted and the two components derive from
core. Also fixes the casing (SAM, not Sam).
- Query/TryGetDateTime.cs -> SAM.Core.Query (SAM 3b16ca4f). The end-of-interval
normalisation is not OpenStudio-specific and belongs next to Query.HourOfYear,
which IntervalHourOfYear already built on. Call sites in Query/DateTime,
Query/TimeIndexDictionary, Create/SpaceSimulationResults and the C5 timestamp
tests now use Core.Query; behaviour is unchanged and the tests stay here, where
they pin the EnergyPlus convention this repo depends on.
- Classes/OpenStudioDiagnosticCodes.cs -> Variables/, matching
SAM/SAM.Core/Variables (Link, Tolerance, Rounding): const values live in
Variables, not Classes. File move only - the namespace is unchanged, so no
caller is affected.
Full solution builds (Rhino closed); 213 passed, 2 skipped.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix: honour the Adiabatic panel flag in the surface boundary condition
An adiabatic floor exported as Ground: the boundary condition was resolved from
PanelType alone, so PanelParameter.Adiabatic (and the zero-thickness-construction
convention behind Analytical.Query.Adiabatic) was ignored and those floors stayed
ground-coupled in EnergyPlus.
New Panel overload of Query.OutsideBoundaryCondition checks Adiabatic() first and
falls back to the PanelType mapping, mirroring SAM_LadybugTools
(New/BoundaryCondition.cs), which tests the same flag ahead of external/underground.
Air and Shade panels are never adiabatic, so an Air panel still pairs as Surface
and carries the shared air-boundary construction.
On an internal panel the flag also drops the adjacent-surface pairing - both sides
become Adiabatic, which is the correct EnergyPlus model of an adiabatic partition
and not a pairing failure.
Tests: flag overrides the PanelType mapping (and an explicit false falls back),
Air is never adiabatic, adiabatic floors convert with NoSun/NoWind and leave other
surfaces untouched, an adiabatic partition drops the pairing without raising the
Surface-branch warnings, and an Air panel converts to a shared
OS:Construction:AirBoundary on both paired sides.
218 passed, 2 skipped.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat: opt-in inter-zone air mixing across Air panels
An OS:Construction:AirBoundary always groups its two zones for solar, daylighting
and radiant exchange, but EnergyPlus defaults Air Exchange Method to None, so no
air moves and the zone air temperatures were coupled only by radiation - an
under-model of a real opening.
New OpenStudioConversionOptions.AirBoundaryAirChangesPerHour (default NaN) switches
the boundary to SimpleMixing at the given rate. It is deliberately opt-in and never
inferred: SAM carries no per-panel airflow data (PanelParameter has no airflow
member), so the rate is the caller's assumption and is named in a diagnostic when
applied. EnergyPlus applies it to the smaller zone's volume on an always-on
schedule; its own documented default for the field is 0.5 ACH. The default NaN
reproduces today's output exactly, so no existing result moves - but that branch is
now diagnosed too, rather than silently uncoupled. A negative or infinite rate is
rejected with a warning and None kept, never clamped; 0 stays an explicit caller
choice, not "unset".
Tests use the production model the features came from, pinned at
tests/resources/models (2 cells joined by an Air partition, both ground slabs
flagged Adiabatic, 10 windows, embedded weather + design days): adiabatic slabs
convert with no surface left ground-coupled, the Air partition pairs and carries
the shared air-boundary construction, the default is None and diagnosed, an
explicit rate reaches SimpleMixing, 0 is honoured, an invalid rate is rejected, and
an end-to-end EnergyPlus run confirms SimpleMixing at 0.5 ACH reaches the generated
IDF and completes with 0 severe errors.
Measured on that model, mixing barely moves the result (25877.1 -> 25872.9 kWh
heating at 5 ACH) because both cells are Ideal-Loads-conditioned to the same
setpoints; it matters when setpoints differ or a zone is unconditioned.
224 passed, 2 skipped.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat: PixelCounting default, caller measures and additional IDF strings
- OpenStudioConversionOptions.ShadingCalculationMethod defaults to
PixelCounting (no concavity limitation: clears the E+ severe
DetermineShadowingCombinations non-convex casting-surface errors);
PolygonClipping or null restores/leaves the OpenStudio default
- OpenStudioConversionOptions.MeasurePaths become OSW workflow steps in
order (arguments at measure defaults); missing dirs warn (SAM-OS-SET-001)
- OpenStudioConversionOptions.AdditionalIdfStrings inject verbatim IDF
objects through a generated EnergyPlus measure appended last; an
unparseable string fails the run - never silently dropped
- SAMAnalytical.ToOpenStudio gains measures_ and add_str_ list inputs
(appended last to preserve archived-document wiring), v1.3.0
- Tests: shading-method default/override/invalid, workflow composition,
add_str end-to-end into in.idf and the invalid-string run failure
* ci: resolve the parent sow quarter branch for dependency clones on push events
A push to a feature branch like sow/2026-Q3-foo has no head/base ref, so
dependency repos fell back from the (non-existent) same-named branch
straight to sow/2026-Q2 - missing APIs introduced in sow/2026-Q3 (e.g.
SAM Core Query.TryGetDateTime) and the build failed with CS0117. Extract
sow/yyyy-Qn from any of head_ref/base_ref/ref_name as a clone candidate.
* ci: fix repo-variable shadowing in the dependency clone loop
The quarter-branch extraction loop reused \ - the outer loop's repo name -
so after it ran, every ls-remote/clone targeted a repo named after the
branch. Rename the inner loop variable.
* fix: reject EnergyPlus-degenerate slivers and name non-convex casting surfaces
Two of the three EnergyPlus severe errors seen on real models:
1. 'GetSurfaceData: 2 degenerate surfaces' - a 1.6 mm sliver door passed
the converter's own cleaning (1e-6 m) and minimum-area check, then
collapsed below three sides under the EnergyPlus 0.01 m
vertex-proximity rule. New Query.EnergyPlusSideCount mirrors that
rule; panels, apertures and shading panels it would degenerate are
skipped at conversion with a warning naming the source object -
EnergyPlus never sees them.
2. 'DetermineShadowingCombinations: casting surfaces non-convex' - new
SAM-OS-GEO-003 advisory names the exact SAM panels (Guid) so they can
be split into convex parts: Warning under PolygonClipping, Information
under PixelCounting (no concavity limitation, but a GPU-less machine
silently falls back to PolygonClipping). Convexity is checked on the
cleaned boundary with a collinear-tolerant test (Query.IsConvex) -
raw CAD vertex streams false-positive on every collinear mid-edge
point; verified to agree with EnergyPlus surface-by-surface on the
reproduction model.
3. 'ReadEPlusWeatherForDay / Out of Range' is fixed in SAM core
(SAM.Weather DataString, commit on sow/2026-Q3): missing atmospheric
pressure was exported as 0 Pa; now the standard-atmosphere pressure
at the site elevation.
Validated end-to-end on the reproduction model: degenerate and weather
severe errors eliminated; the two non-convex panels are now named by
Guid (the severe itself requires convex splitting or a GPU).
* feat: add 'Go to Directory' context menu and normalise GH outputs
Right-click 'Go to Directory' on all five OpenStudio Grasshopper
components (same behaviour as SAMAnalytical.Paths in the SAM repo):
ToOpenStudio opens _outputDirectory, RunModel opens outputDirectory_
(falling back to the folder of _path), and the SQL components open the
containing folder of _sQLPath. Shared volatile-path/open logic lives in
the new internal MenuHelper.
Output convention: RunModel moved 'successful' from first to last so
'diagnostics, successful' are always the last two outputs; AddResultsBySQL
renamed 'Successful' to lowercase 'successful'. Component versions bumped
(RunModel 1.2.0, ToOpenStudio 1.3.1, AddResultsBySQL 1.0.6, both SQL
query components 1.0.3).
* B0 audit: document OpenStudio-to-SAM mapping and baseline
Records the pre-implementation audit for the reverse (OpenStudio -> SAM)
conversion:
- verified baseline on sow/2026-Q3: x64 solution build succeeds, the test
suite is 237 total / 235 passed / 2 skipped / 0 failed;
- the existing infrastructure that must be reused rather than duplicated
(OSM loader + VersionTranslator, CLI discovery, async run with
process-tree termination and unique run directories, diagnostics,
statistics, object map, geometry validation, naming);
- the OpenStudio 3.10.0 .NET API surface confirmed by reflecting the
referenced assembly - notably AdditionalProperties on every ModelObject
and the group-local vertex convention of PlanarSurfaceGroup;
- the full reverse-mapping matrix (model/site, spaces/zones, surfaces,
subsurfaces, shading, materials/constructions, loads/schedules) with a
Native/Derived/Approximated/Unsupported/Deferred status, diagnostic code
and test reference per row;
- the SAM-OSI-* diagnostic code list;
- the identity audit: the forward exporter stores NO full SAM GUID today
(no additionalProperties usage anywhere in the repository), so the
8-character name suffix cannot reconstruct one - namespaced
SAM.Guid/SAM.Type/SAM.Name features will be added non-breakingly;
- the OSW seed-versus-executed behaviour contract and the deterministic
final-OSM discovery order.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* B1 core: add OSM/OSW import options, context, result and diagnostics
Reverse-direction scaffolding, kept deliberately separate from the forward
conversion types so neither direction's contract moves when the other grows:
- OpenStudioImportDiagnosticCodes: the SAM-OSI-* code list from the audit
(input, OSM load/version, OSW parse/seed/execution/final-OSM, geometry,
adjacency, boundary condition, subsurface, material, construction,
schedule, load, HVAC, zone, identity, weather, settings, approximation).
Disjoint from the forward SAM-OS-* codes.
- OpenStudioImportOptions: tolerances, feature switches, geometric-adjacency
fallback, identity restoration, workflow execution.
- OpenStudioImportContext: source model, caches keyed by OpenStudio object
name, statistics, RegisterOnce, and the identity ledger. ResolveGuid
restores a stamped full SAM Guid, rejects malformed/mistyped/duplicate
metadata with SAM-OSI-ID-001 and issues a fresh Guid instead, and stays
silent for a third-party OSM that carries no metadata at all.
- OpenStudioImportResult: AnalyticalModel, SourcePath, ResolvedOsmPath,
OpenStudioVersion, RunResult, Diagnostics, Statistics, IsValid,
Successful. Not disposable and never exposes a live native model - the
loaded model is disposed inside the import.
Identity metadata (audit finding: no full SAM GUID was persisted anywhere):
- OpenStudioIdentityKeys (SAM.Guid / SAM.Type / SAM.Name) and
Core.OpenStudio.Modify.SetSAMIdentity / Query.TryGetSAMGuid|Name|Type|Feature
over OS:AdditionalProperties.
- The forward converter now stamps identity at the single point every
registered SAM object passes through (OpenStudioConversionContext
.RegisterModelObject) plus the Building for the model itself. This is
additive - AdditionalProperties is a separate OSM object, so no field,
object name or EnergyPlus input changes and the released
SAMAnalytical.ToOpenStudio contract is unaffected.
Create.Model gains an overload reporting the pre-translation OSM version,
the VersionTranslator errors/warnings and a single-line failure reason, so
a load failure can become an actionable diagnostic instead of a bare null.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* docs(diagnostic): clarify SAM-OS-GEO-003 does not detect GPU absence
The non-convex-casting advisory read as though the converter had
determined the machine has no GPU, and the code comment claimed
EnergyPlus "silently" falls back. Neither is accurate: the converter
never queries for a GPU (the advisory is unconditional for non-convex
casting surfaces under PixelCounting), and per the EnergyPlus I/O
Reference EnergyPlus WARNS in eplusout.err before reverting to
PolygonClipping when no GPU/GPU-emulation is available.
Reword the PixelCounting Information message, the PolygonClipping
Warning message, the doc comment and the test comment accordingly.
Message still contains "GPU" so the C4 assertion holds. No behaviour
change.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* B2 geometry: convert spaces, surfaces, subsurfaces and shading
Reverse geometry converters, all sharing one validation policy and never
repairing what they cannot convert.
SAM.Geometry.OpenStudio:
- Convert/ToSAM/Point3D.cs: Point3d and Point3dVector to SAM, with an
optional group transformation. OpenStudio surface vertices are stored in
their PlanarSurfaceGroup's coordinate system, not in world coordinates -
applying the group transformation is what stops a third-party OSM with
non-origin spaces importing as a pile of overlapping geometry. SAM-authored
OSMs place every space at the origin, so round trips are unaffected.
- Convert/ToSAM/Face3D.cs: PlanarSurface to a validated Face3D through the
existing forward ValidatePolygon/CleanVertices, re-badging its diagnostics
with the SAM-OSI-GEO-001 import code. OpenStudio's winding is preserved
because the importer classifies floors versus ceilings from that normal.
ToSAM_Normal recomputes the outward normal from transformed vertices
rather than trusting the group-local outwardNormal().
SAM.Analytical.OpenStudio:
- Query.SAMPanelType: PanelType from surface type AND boundary condition
together (a "Wall" is external, internal or underground depending on the
latter); the whole Ground/Foundation/preprocessor family maps to ground;
the "other side" conditions fall back to adiabatic and report
SAM-OSI-BC-001. Adiabatic keeps the internal panel type and records the
flag separately, matching how the forward direction reads it back.
- Query.SAMApertureType: subsurface type to ApertureType, flagging the
mappings that discard behaviour (operable windows, tubular daylighting,
overhead doors) so they report SAM-OSI-APX-001 instead of looking exact.
- Convert/ToSAM/Panel.cs, Aperture.cs, Space.cs, ShadingPanels.cs.
Apertures attach through Panel.AddAperture so SAM validates containment.
Spaces derive area/volume/internal point from their own closed shell and
fall back to the OpenStudio values only when no shell can be built; one
OpenStudio space always becomes one SAM space, with a multi-space
ThermalZone reported (SAM-OSI-ZONE-001) rather than collapsed. Shading
composes the group transformation with the owning space's building
transformation for Space-type groups.
- OpenStudioSourceParameter + Modify.SetOpenStudioSource record the source
name, handle and IDD type on every imported object. A SAM Panel's Name is
structural (it comes from its Construction), so provenance cannot live
there; the handle is also the only source identity that is truly unique.
Constructions resolve from the context maps that B4 will populate, falling
back to a name-only placeholder with no invented layers - a missing build-up
stays visible instead of being replaced by a default.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* B4 libraries: convert constructions and materials
Inverse of Convert/ToOpenStudio/Material.cs and Construction.cs, following
docs/SAM_OPENSTUDIO_MATERIAL_MAPPING.md so a round trip is exact where the
two models genuinely agree, and reported where they do not.
Materials (deduplicated by OpenStudio name into a SAM MaterialLibrary):
- StandardOpaqueMaterial -> OpaqueMaterial. EnergyPlus opaque materials are
single-sided, so both SAM sides receive the same absorptances rather than
an invented asymmetry.
- MasslessOpaqueMaterial -> OpaqueMaterial at a nominal 25 mm with
conductivity = thickness / R. The resistance EnergyPlus used is preserved
exactly; only the absent thermal mass is assigned, as near zero as SAM
allows, and always reported.
- AirGap -> GasMaterial with h = 1/R, the exact inverse of the forward
R = 1/h, so an air cavity survives a round trip unchanged.
- StandardGlazing -> TransparentMaterial with Front/Back mapped back to
External/Internal exactly as the forward direction writes them, so the
glazing is not mirrored.
- SimpleGlazing -> a single reconstructed glazing layer preserving U, with
solar transmittance taken from the SHGC; reported, since SHGC includes
inward-flowing absorbed radiation and so overstates transmittance.
- Gas / GasMixture -> GasMaterial from the 20 C conductance; a mixture's
composition cannot be carried and is named in the diagnostic.
- Anything else (blinds, shades, screens, thermochromic, refraction/
extinction glazing) -> a name-only placeholder plus SAM-OSI-MAT-001. A
plausible-looking default would silently change the model's physics; an
empty material is visibly incomplete.
Constructions:
- Classified from their layers, not from the surfaces that use them, so the
opaque/fenestration decision is independent of import order.
- Layer order preserved outside-first, matching what the forward direction
writes.
- ConstructionAirBoundary -> a layer-free construction (PanelType.Air is
carried on the panel); ConstructionWithInternalSource, CFactor/FFactor
constructions and window data files -> SAM-OSI-CON-001 with a name-only
construction.
- Resistance-defined layers report zero thickness in OpenStudio; the nominal
thickness recorded by the material converter is used so no SAM layer is
zero-thickness.
- Aperture constructions carry pane layers only - OpenStudio keeps frame and
divider in a separate object, so SAM frame layers would be fabricated.
Modify.RemoveUnreferencedMaterials / RemoveUnreferencedProfiles prune what
the imported topology does not reference, mirroring the SAM_LadybugTools
reverse converter: an OSM routinely carries whole template libraries that
would otherwise drown the handful of entries the building actually uses.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* B5 conditions: convert loads, controls, schedules and profiles
Schedules -> SAM profiles:
- Everything is expanded to 8760 hourly values, which is what a SAM Profile
is and the exact inverse of the forward direction (SAM profiles are written
out as 8760-value ScheduleFixedIntervals).
- ScheduleConstant, ScheduleRuleset and ScheduleInterval are supported. A
ruleset is expanded through OpenStudio's own getDaySchedules over the whole
year, so weekday/weekend rules, seasonal date ranges and rule priority are
resolved by the engine that defines them instead of being reimplemented -
and the schedule is never flattened to a single constant.
- Days are sampled at the end of each hour, the EnergyPlus "Until: HH:MM"
convention, so a 09:00-17:00 pattern is not shifted by one hour.
- Sub-hourly series are averaged (not sampled) into hourly means so a
fractional schedule keeps its daily total; short series are tiled; both a…
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
Quarterly Q2 2026 work from the SAM-BIM fork merging back into the upstream.
Includes the Newtonsoft.Json → System.Text.Json.Nodes migration of the
entire SAM-BIM stack (per-repo PRs already merged into SAM-BIM's
sow/2026-Q2).Highlights of this quarter
IJSAMObjectimplementations migrated fromJObject/JArray/JTokentoJsonObject/JsonArray/JsonNode.FromJObject/ToJObjectrenamed toFromJsonObject/ToJsonObject.System.Text.Json10.0.8 replacesNewtonsoft.Json13.0.3 in every csproj(except SAM_Revit which keeps Newtonsoft loadable for Revit's runtime).
Log identity loss, SearchWrapper missing type discriminator, RelationCollection
wrong-jObject-passed).
Verification
BuildAlls_v3 RestoreCleanRebuildagainst the merged sow/2026-Q2 chain:0 errors, 144 artifacts.
Test plan
IJSAMObjectAPI renamemasterfrom upstream🤖 Generated with Claude Code