Repository navigation
sow/2026-Q3: SAM-BIM Q3 2026 updates - #8
Merged
Merged
Conversation
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
* 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>
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>
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>
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>
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>
…eview 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>
…ts (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>
…(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>
…eview 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>
…ks (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>
…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>
… 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>
… (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>
…s 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>
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.
…ersion 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.
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.
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.
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.
…erture 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>
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>
…lidation Reject implausible SAM design-day barometric pressure
Test totals: 237 -> 309 (307 passed, 2 skipped as before, 0 failed). R1 OSM import (17): every fixture is exported and re-imported in memory, so the two directions are held against each other. Covers one closed room; two adjacent rooms whose shared wall must become ONE panel related to TWO spaces (11 panels, not 12); stacked storeys; floor/roof/external/internal/ground panel types; windows and their absence; volume and aperture area within tolerance; construction layers with no zero-thickness layer and every material present in the library; pruning of unreferenced materials; the air-gap h = 1/R round trip being exact; internal conditions, profiles and setpoint profiles carrying the fixture's 21 degC; the north axis; and an empty model, which is an empty building rather than a failure. R2 OSW import (17): seed-only with the "measures NOT applied" warning; relative seed resolution through <root>/files and file_paths; missing seed, absent seed with execution disabled, malformed JSON, a JSON array, a missing path, an unsupported extension and a corrupt OSM - each a specific blocking diagnostic and never a silently empty model; OSM import needing no EPW; isolated run directories and verbatim step/argument preservation when staging; final-OSM discovery preferring the recorded osm_path, falling back to run/in.osm, and ignoring an OSM older than the run; and cancellation returning diagnostics rather than throwing. R3 identity (11): the forward converter stamping full Guid/type/name; restoration of space, panel and model-level Guids; restoration switched off; a third-party OSM built through the OpenStudio API importing cleanly with no identity diagnostics; malformed, duplicated and wrongly typed metadata each reported and replaced with a fresh Guid; and the 8-character name suffix explicitly NOT treated as a restorable identity. R4 shading and diagnostics (19): site/building/space shading with the space transformation composed; space-relative geometry; unsupported boundary conditions; a dangling interzone adjacency (produced by editing a saved OSM, because OpenStudio refuses that state in memory); operable windows, glass doors and subsurface multipliers; SimpleGlazing and massless materials with resistance preserved exactly; a surface with no construction; unsupported load kinds; detailed HVAC; multi-space zones; site and design days; an unset site producing no Location; below-minimum-area rejection; a ScheduleRuleset expanding to 8760 values that still contain BOTH the default and the weekend rule value; and an unexpandable schedule leaving the load with no profile. R5 coverage enforcement (8) over tests/resources/openstudio-to-sam-coverage.json: structural soundness, every manifest diagnostic being a real constant, every constant being claimed by an entry, the SAM-OSI- prefix, disjointness from the forward code set, every PanelType being reachable in reverse, the whole subsurface-type domain mapping or being explicitly unsupported, and every value in OpenStudio's own validOutsideBoundaryConditionValues() being classified rather than silently Undefined. Product fix found by these tests: identity was stamped only on the PRIMARY surface of an internal panel, so whether a shared wall kept its SAM Guid depended on which of its two surfaces the importer met first - and OpenStudio's handle ordering is not stable between runs. Both Panel and Aperture now stamp every side; the traceability map stays once-per-Guid. A [Repeat(10)] test guards it. Also from these tests: the Site guard now keys on coordinates alone. Every OpenStudio model has an auto-named Site, so requiring an empty name meant a default (0,0,0) site was imported as a location in the Gulf of Guinea. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- docs/SAM_OPENSTUDIO_TO_SAM_COVERAGE.md: the reverse coverage tables, generated from tests/resources/openstudio-to-sam-coverage.json and grouped by domain. 141 entries: 55 Native, 10 Derived, 17 Approximated, 45 Unsupported, 9 Deferred, 5 NotApplicable. The tests enforce the manifest, never this Markdown. - docs/openstudio-to-sam-usage.md: the Grasshopper component contract, the API, the OSM-versus-OSW behaviour table, round-trip identity, what survives a round trip, and the limitations - separated into what is reported but not imported, what is approximated and said so, what is not embedded (a WeatherFile names an EPW, it does not contain one), and why geometry is never repaired. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…real models Two regressions found importing a real multi-zone SAM-exported building in Rhino 8; neither was reachable from the synthetic fixtures. 1. Opaque-pane aperture constructions (solid doors) lost their identity AND their construction. The forward converter writes an ApertureConstruction with opaque panes as an OS:Construction with opaque layers - byte-identical in shape to a wall construction, distinguished only by the stamped SAM.Type=ApertureConstruction feature. The reverse classifier decided opaque-vs-fenestration from layer materials alone, so it built these as Construction: the type-guard then rejected the GUID (SAM-OSI-ID-001) and, worse, filed them in ConstructionMap instead of ApertureConstructionMap, so the door's subsurface fell back to an empty placeholder construction. Fix: Convert/ToSAM/Construction.cs now treats a construction as fenestration when its stamped SAM.Type says ApertureConstruction, regardless of layer materials. The door keeps its real construction and its GUID, and no collision is reported. 2. Real spaces lost their SAM location. Space.ToSAM computed the internal point with Shell.InternalPoint3D at DistanceTolerance (1e-6 m); real exported geometry only closes at the millimetre, so the shell was not watertight there and five spaces got a null location (reported as SAM-OSI-ZONE-002). A null Space.Location breaks every downstream space-in-point query and label. Fix: the shell operations now run at MacroDistance (1e-3 m) - the tolerance the forward direction already uses - with a CalculatedInternalPoint3D fallback and, if the shell still cannot close, the vertex centroid (reported as SAM-OSI-APX-001, since a centroid may fall outside a concave space). A space now always gets a location. Tests (R6, 4 new; suite 309 -> 313, 311 passed): the opaque-pane door round-trips as an ApertureConstruction with its GUID and real layer and no identity collision; a box that closes only at 0.5 mm still gets an internal-point location and a correct volume; an unclosable (roofless) space falls back to a centroid location with the approximation reported, never null; and a five-room building has a location on every space with no "location is not set" warning. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The importer recorded only the weather-file path, so a re-imported model had no weather to show. When the OS:WeatherFile reference resolves to an EPW that exists on disk - which it does for a SAM round trip, whose forward export writes the file - its hourly weather is now loaded into SAM WeatherData (Weather.Convert.ToSAM) and embedded in the AnalyticalModel under AnalyticalModelParameter.WeatherData, the same slot the forward exporter reads back. The weather round trip is now closed: AnalyticalModel(WeatherData) -> OSM(WeatherFile ref + temp EPW) -> AnalyticalModel(WeatherData) -> EPW. - OpenStudioImportOptions.ImportWeatherData (default true) gates it. - The context now carries SourceDirectory so a weather path recorded relative to the OSM resolves; a file:/file:// scheme prefix is stripped and forward slashes normalised. - A file with no hourly years is treated as a header-only reference, not weather. When the EPW is absent or import is disabled, only the path is recorded (OpenStudioSourceParameter.WeatherFilePath) and the diagnostic says no weather was embedded - a bare reference is never presented as embedded weather. Model.WeatherFile is reclassified Native->Derived in the manifest. Tests (R7, 4 new): a referenced EPW on disk is loaded into embedded WeatherData with hourly years; that embedded weather re-exports to an EPW via EmbeddedAnnualWeatherPath (full loop); import disabled keeps only the path; and the metadata path is recorded regardless. Also stabilised the R4 UnpairedInterzoneSurface test, which was flaky (1-in-3): it blanked only the first interzone adjacency line, and OpenStudio's surface serialisation order varies between runs, so it sometimes edited a surface that still paired. It now blanks every interzone adjacency reference and disables the geometric fallback, making the adiabatic-demotion outcome deterministic (verified 5/5). No product code involved. Suite 313 -> 321 (319 passed, 2 skipped, 0 failed). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…gnostics Importing the real building produced ten SAM-OSI-GEO-001 *warnings* that were all just vertex cleaning - "Removed 1 duplicate/collinear vertices (5 -> 4)". Removing a collinear vertex leaves the polygon's shape, area and normal identical, so these are routine normalisation of geometry OpenStudio happens to store with redundant points, not defects. Reporting them under the same code and severity as genuinely unconvertible geometry buried the real failures and made a clean import look alarming. The forward direction already separates these (SAM-OS-GEO-002 cleaning vs SAM-OS-GEO-001 invalid); the import had collapsed both into one code. Added SAM-OSI-GEO-002 (GeometryVerticesCleaned) and mapped the cleaning diagnostic to it at Information severity, leaving every genuine failure at its original code and severity. Docs corrected alongside: the WeatherFile row is now Derived (hourly weather is embedded when the EPW is on disk, per B11) rather than "reference metadata only", and the space-shell row now names SAM-OSI-APX-001 for the centroid fallback rather than the geometry code it no longer uses. Tests: one new R6 test asserts a redundant mid-edge vertex is reported under GEO-002 at Information, is NOT reported as invalid geometry, and that the panel still imports with its true area. Suite 321 -> 322 (320 passed, 2 skipped, 0 failed); manifest enforcement still green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…mport Importing a real executed OSW reported [SAM-OS-RUN-001] Error: OpenStudio CLI run failed (exit code 0, SQL missing, 0 fatal error(s)) and then imported the model perfectly. The CLI had exited 0 with no fatal errors; the workflow simply ran model measures and produced no EnergyPlus results. The Error came from the shared OpenStudioSimulationRunner, which judges a run by whether eplusout.sql exists - the right test for a simulation and the wrong one for an import, which needs the final OpenStudio model and nothing else. Because that Error landed in the import's diagnostics, IsValid returned false, so the Grasshopper component's successful output was false and it showed an error, despite a valid model having been imported. That is a correctness bug in the OSW path, not cosmetic. The runner's diagnostics are now collected separately and folded in through AppendRunDiagnostics, which re-badges exactly one verdict: when the workflow DID produce a final model, the CLI exited 0 and nothing was fatal, the RunCliFailed error becomes an informational SAM-OSI-OSW-009 explaining that an import does not need simulation results. Everything else - real CLI failures, fatal errors, EnergyPlus severe errors, cancellations - passes through untouched at its original severity, so no CLI diagnostic is lost. Also fixed a code/message contradiction: the "the workflow WAS executed" message was raised under OswWorkflowNotExecuted. Added the distinct SAM-OSI-OSW-008 (OswWorkflowExecuted) so the code alone tells a reader whether the measures were applied. Tests (3 new in R2): a results-less clean run that produced a model leaves no Error and reports OSW-009; a genuine failure with no model keeps its Error; and EnergyPlus severe errors pass through unchanged. Suite 322 -> 321 total (319 passed, 2 skipped, 0 failed - one earlier count included a duplicate fixture run). Docs and coverage manifest updated (144 entries). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…and errors Validating against a real executed workflow showed the run directory containing a 44 MB eplusout.sql and a finished.job, while the import reported "SQL missing". Cause: WriteIsolatedCopy set "run_directory" in the staged OSW copy. The OpenStudio CLI honours that field and writes EVERY output - eplusout.sql, eplusout.err, in.osm, out.osw - into that directory's ROOT, with no run/ subdirectory. OpenStudioSimulationRunner reads them from <osw>/run/, so it found neither the results nor the error file. Two consequences, the second serious: - RunResult.SqlPath was null and the run was judged failed even though it succeeded. - eplusout.err was never parsed, so EnergyPlus severe errors were silently lost. On the validation model that discarded a real report of 2 severe errors (including DetermineShadowingCombinations non-convex casting surfaces) and 42 warnings - exactly the diagnostics the spec requires to be preserved. B13 downgraded that "SQL missing" verdict to information, which masked this rather than fixing it. B13 remains correct for a genuinely results-less workflow (model measures with no simulation step) but will now stop firing spuriously on real runs. Fix: do not set run_directory, and strip any the source OSW carried. Isolation comes from staging the copy in a unique directory - the CLI's default <osw>/run/ then lands inside it, which is exactly where the runner looks. FindFinalOsm already handled both layouts. The existing parallel-isolation test asserted "Each copy must pin its own run directory" - a test locking in the defect. Corrected, and a new IsolatedCopy_DoesNotPinRunDirectory regression test asserts the copy strips a run_directory even when the source OSW sets one. Suite 321 -> 322 (320 passed, 2 skipped, 0 failed). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Correction to B14, found by running the full four-leg workflow matrix (SAM->OSM, SAM->OSW, OSM->SAM, OSW->SAM) end to end against a real 14-space model. B14 removed the run_directory pin; that turned out to be the wrong half of the fix. There are three possible settings and only one works: - run_directory unset (B14): the CLI resolves the run folder against "root", which points at the ORIGINAL workflow's directory. The output escapes the staged directory entirely - so isolation is lost AND the final OSM is not found. The executed leg returned no model at all. - run_directory = <staged> (B6, original): the CLI writes eplusout.sql, eplusout.err and in.osm into the staged directory's ROOT, with no run/ subdirectory, while OpenStudioSimulationRunner reads them from <working>/run. Results and the EnergyPlus error report were silently missed. - run_directory = <staged>/run (this commit): the output stays inside the staged directory (isolation) and lands exactly where the runner looks (diagnostics preserved). Verified end to end on the real model: the executed OSW leg now reports success=True, isValid=True, RunResult.Success=True, a populated SqlPath and ErrorFilePath, and - the point of the whole exercise - 1 EnergyPlus severe error surfaced as SAM-OS-EPLUS-001 (DetermineShadowingCombinations non-convex casting surfaces) that both earlier variants discarded. The imported model is identical to the other three legs: 14 spaces, 84 panels, 41 shared panels, all space/panel/model GUIDs preserved. SAM-OSI-OSW-009 correctly stops firing now that results exist. The regression test is rewritten to assert the exact run_directory value rather than its presence or absence, with the two failure modes documented so neither wrong variant can be reintroduced. Suite 322 (320 passed, 2 skipped, 0 failed). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Resolves the REQUEST_CHANGES review on PR #14 (four inline P1 blockers): 1. Native crash in the adjacency fallback. Unpaired interzone surfaces were retained as native SWIG Surface wrappers and dereferenced after the model walk (nameString()), which aborts testhost with an unmanaged AccessViolationException under GC pressure. Snapshot the managed name and diagnostic label during traversal (UnpairedInterzoneSurface) and never touch a native surface afterwards. 2. Duplicate panels after geometric pairing. The fallback updated only panelBySurfaceName, leaving the superseded panel in the already-captured space->panel lists, so Pass D added two coincident panels (12 instead of the shared-panel 11). Record supersessions and remap+dedupe the captured lists before building topology. 3. Lost layers for metadata-free opaque doors. A solid door exports as an OS:Construction with opaque layers and, absent a SAM.Type stamp, is filed in ConstructionMap; the aperture resolver missed it and produced a layerless placeholder. Recover the opaque layers onto a SAM aperture construction on demand (reported once as SAM-OSI-APX-001), keyed off the subsurface's use. 4. Incorrect expansion of non-hourly fixed-interval schedules. A 365-value daily series was tiled by value index, ramping the first day 1,2,3,... instead of holding each daily value across its 24 hours. Resample from the TimeSeries interval: hold coarse values, average sub-hourly, handle partial and leap-year coverage explicitly, and report a variable-interval schedule as unsupported rather than guessing. Regression tests: R4 geometric-fallback shared topology (11 panels), a repeated dangling-adjacency import guarding the native crash, daily fixed-interval hold semantics and variable-interval-unsupported; R6 metadata-free opaque door keeps its layers. Debug and Release x64 build clean; full suite green (0 failures). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
feat: import OpenStudio OSM/OSW into SAM AnalyticalModel (reverse conversion)
Clone SAM_Validation as a sibling (same branch-candidate fallback as the existing deps clone) and build only SAM.Analytical.Benchmark.csproj into its build\ folder, before the SAM_OpenStudio solution is built. The B1b OpenStudio benchmark producer references that flat DLL by HintPath, so it must exist first. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
New SAM.Analytical.OpenStudio.Benchmark project (net8.0, x64) with the engine-neutral mapping from a simulated SAM AnalyticalModel + run context to a B1a schema-v1 BenchmarkDocument (route Native-OpenStudio). - OpenStudioBenchmarkContext carries the C5 result set and the run provenance (both model hashes, weather identity/hash, engine/SDK versions, commits, timing, state) as SAM-neutral inputs — no OpenStudio artefacts. - ToBenchmark reads measurements through the established SAM mappings (OpenStudioSimulationResultSet.ToSAM / ToSAM_SpaceSimulationResults): annual consumption in kWh exactly as the one J->kWh authority produced it, model peaks in kW, space peaks/design loads in W, hours as hour-of-year 0..8759. Availability follows the schema invariant; a peak with no valid hour is emitted unavailable, never a misleading zero. Schema DLL referenced by flat HintPath (three ..) matching the repo's existing SAM\build convention. No System.Text.Json package (net8.0 shared framework). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Program.cs: the `benchmark-openstudio --model --weather --out` CLI. Computes sourceFileHash over the raw model bytes and canonicalModelHash over the neutral SAM model BEFORE any OpenStudio translation (B1a helpers, exactly per SCHEMA.md), runs the headless ToOpenStudio route, maps via ToBenchmark, validates and writes. Exit codes: 0 success, 1 unexpected, 2 usage, 3 input, 4 run failure, 5 invalid. Commits resolve from --sam-commit/--runner-commit, env, InformationalVersion SHA or git HEAD. - Offline tests (no OpenStudio/EnergyPlus): golden mapping of the C5 result set to the document (model + space metrics, explicit units, measured-zero unmet hours, unavailable design loads), Native-OpenStudio provenance, zero-error/zero-warning validation, serializer round-trip, and the two-hash contract (source varies with formatting, canonical stays stable, never equal). - Both projects added to SAM_OpenStudio.sln (x64). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
[Category(\"Simulation\")] end-to-end test: runs the headless OpenStudio route on the SingleBox fixture, reads the raw annual Ideal Loads heating energy from the EnergyPlus SQLite (Joules), and pins the stored benchmark magnitude to kWh (J / 3 600 000, the one J->kWh authority) rather than Wh (J / 3600). Self-ignores when no OpenStudio CLI is installed. Empirically observed on this run: raw 10 529 306 275.33 J -> stored 2924.807 kWh (Wh would have been 2 924 807.3). This is the gating unit decision; the B2 TAS producer must store consumption in the same kWh magnitude to stay comparable. The fixture gains a full simulatable SingleBox (same box, GUID and name as C5). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Addresses the Codex review on PR #16. 1. Blocking — use the shared benchmark CLI contract. Program.cs no longer has its own parser and exit-code enum; it delegates to BenchmarkCliHost.Run with BenchmarkArguments/BenchmarkCliPaths, so it inherits invariant culture, exception mapping and the canonical exit codes: 0 success, 2 usage, 3 input/ IO/serialization, 4 validation, 5 producer failure. --help now prints usage once and returns success; codes 4/5 are no longer reversed. A non-model input file is reported as a JsonException (exit 3), not a producer failure. 2. Release gate — CI now runs the portable producer tests. New build.yml step runs `dotnet test --filter TestCategory!=Simulation`; the live EnergyPlus test stays environment-gated. 3. Plan compliance — committed golden + CLI tests. Added a byte-for-byte golden fixture (golden-openstudio-benchmark.json, pinned to LF via .gitattributes so the comparison is cross-platform) asserted against the serializer output, and CLI tests covering --help, missing option, missing/unparseable input, and the validation/producer/success exit decisions (via an internal Emit seam, all offline). The fixture context is now fully deterministic so serialization is byte-stable. Regenerate intentionally with SAM_BENCHMARK_UPDATE_GOLDEN=1. Verified locally: producer + tests build clean; offline suite 14/14 pass, byte-stable across repeated runs. Generated by Michal Dengusiak and CodeClaude Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
B1b — OpenStudio benchmark producer CLI
The OpenStudio benchmark producer wrote provenance.designDaySource = None unconditionally. The B3 comparator compares that field between the TAS and OpenStudio documents before deriving a gate status, and the TAS producer derives the real value, so a genuinely paired cross-engine run on the same model (identical canonicalModelHash) was reported PROVENANCE INCOMPATIBLE purely because of the fixed value: TAS EmbeddedModel vs OpenStudio None on HungaryHouse + HUN_CEN_Budapest TMYx (2026-07-25), even though the OpenStudio run demonstrably had design days. The route already resolves the basis (explicit DDY, then the design days embedded in the AnalyticalModel, then none — never merged) but only reported it as diagnostic text. It is now recorded structurally and read back by the producer, so the field states what the run actually did: - SAM.Core.OpenStudio: new OpenStudioDesignDaySource (None/Ddy/ EmbeddedModel) — the outcome, not the intent. - OpenStudioConversionContext.DesignDaySource, set in lockstep with DesignDaysImported by both ToOpenStudio_DesignDays overloads, and snapshotted onto OpenStudioConversionResult. - The producer derives the pre-run value from the source model exactly as SAM.Analytical.Tas.Benchmark does, then replaces it with the basis the route established; a model that carries design days none of which were imported reports None plus a note, never a false EmbeddedModel. Tests: new offline DesignDaySourceTests in the CI-run producer test project (embedded days -> EmbeddedModel, heating-only, cooling-only, absent/empty/null -> None, route-basis mapping, no Unknown ever emitted); the C4 route tests now also assert the structural field for the embedded, explicit-DDY and no-design-day cases. Verified locally against the real HungaryHouse model: 1 heating + 1 cooling embedded day, route reports EmbeddedModel. Generated by Michal Dengusiak and CodeClaude Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…profile (#18) The implausible-pressure guard added in 52d2840 only writes the barometric-pressure field when the SAM design day HAS a usable pressure: the whole substitution block sits inside `if (!double.IsNaN(pressure) && pressure > 0)`. When the profile is absent, the mean is NaN, the block is skipped, and the field is never written — so it keeps the OpenStudio design-day default, which is the 31000 Pa IDD floor. The comment in that same block warned about exactly this ("the unset OpenStudio default is the 31000 Pa IDD floor, so the field must be written, not left"); the code did not honour it. This is not hypothetical. The B3 benchmark run recorded in SAM_Validation docs/benchmark/reports/2026-07-25-cross-engine-provenance-fixed used HungaryHouse-WeahterDDY.sam, whose embedded design days carry dry bulb, RH, wind and solar but NO pressure. Both SizingPeriod:DesignDay objects in the generated in.idf therefore read: 31000, !- Barometric Pressure {Pa} and EnergyPlus logged, for both days: ** Warning ** SetUpDesignDay: Entered DesignDay Barometric Pressure=31000 differs by more than 10% from Standard Barometric Pressure=99785. ** ~~~ ** ...Standard Pressure (based on elevation) will be used. Fixed by hoisting standardPressure and writing it in an else branch, with a warning diagnostic naming the substitution — the same shape as the implausible-value branch. SCOPE OF THE IMPACT — deliberately not overstated. 31000 Pa is physically absurd (about 9 km altitude; this site is ~100 kPa) and definitely wrong to emit, but it appears to have been RESULT-NEUTRAL at runtime for that run: EnergyPlus detected it and substituted the elevation-based standard itself, as its own warning states. So this is a confirmed input-generation defect and a candidate contributor to cross-engine sizing differences whose runtime impact is NOT established. Establishing it needs the sizing-environment conditions EnergyPlus actually used (eplusout.sql), which belongs with the design-day audit work, not this fix. What is certain: the IDF was wrong and misleading, it relied on an engine-side silent correction, and it produced the confusing EnergyPlus warning this conversion exists to prevent. Tests — the three cases now pinned explicitly: - MissingPressure_WritesElevationStandardNotTheIddFloor (new): removes the fixture's pressure profile via WeatherDay.Remove, asserts the elevation-based standard is written, asserts 31000 never reaches the IDF, and asserts the substitution is named. Verified to FAIL on the pre-fix converter with "Expected: greater than 90000 and less than 110000, But was: 31000.0d". - ImplausiblePressure_RejectedWithDiagnostic (existing): unchanged and still passing. - PlausiblePressure_IsRetained (new): a value within 10% of the standard is converted unchanged and reports no substitution. Suites: 331 passed / 2 skipped in SAM.Analytical.OpenStudio.Tests, 24/24 offline in SAM.Analytical.OpenStudio.Benchmark.Tests. No regressions. Generated by Michal Dengusiak and CodeClaude Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…oo (#19) The producer recorded conversion diagnostics only when the EnergyPlus run FAILED, and the helper it used returned only Severity.Error. A successful benchmark document therefore emitted: "warnings": [], "notes": [] even though the route had explicitly named its own approximations. Both recorded B3 cross-engine runs show exactly that — empty arrays while the conversion had reported the hourly humidity collapsed to a constant dew point, the hourly solar profile replaced by ASHRAEClearSky, a substituted barometric pressure, and the weather/design-day basis. The neutral schema provides warnings/notes precisely so a reader can see what was approximated; a document that hides them presents a comparison as cleaner than it was. New Modify.ApplyConversionDiagnostics runs on EVERY run, before the document is built: - Warning AND Error -> context.Warnings, regardless of run outcome. An Error that did not stop the run is never less relevant than a Warning. - Information -> context.Notes ONLY for an allow-list of stable codes, never by matching message text, so a reworded diagnostic cannot silently change what is recorded: * WeatherDataIssue (SAM-OS-RUN-002) — annual weather source, design-day source, number/identity of imported design days, ground-temperature source. These decide what the run actually simulated and sized against. * ResultExtractionLimitation (SAM-OS-RUN-003) — qualifies the measurements the benchmark itself publishes (e.g. peaks assuming hourly reporting). * SimulationSettingsUnsupported (SAM-OS-SET-001) — a rejected or substituted simulation setting changes the basis the numbers were produced on. Geometry, adjacency, material, construction, schedule, internal-condition and HVAC codes are deliberately excluded: as Information they are per-object modelling detail, there can be hundreds per model, and copying them would turn notes into conversion chatter. Their Warning/Error variants still reach warnings, so nothing material is dropped. - Both arrays are then made byte-stable: null/whitespace-only entries dropped, exact ordinal duplicates dropped, StringComparer.Ordinal sort. Producer-generated notes (EnergyPlus version unavailable, model carried design days but the route imported none, run did not complete) are preserved and normalized together with the propagated ones. Message text is taken verbatim from OpenStudioDiagnostic.ToString(), never rewritten. KNOWN GAP, deliberately not fixed here: the "Site coordinates taken from …" Information diagnostic (Convert/ToOpenStudio/Weather.cs) carries the RunCliFailed code rather than WeatherDataIssue, so it does not reach notes despite being material — site elevation feeds the design-day standard pressure. Re-coding it would change conversion output, which is out of scope for a producer-reporting PR; raised as a follow-up instead of widening the allow-list to a code whose documented meaning is "OpenStudio CLI failed". Nothing else changes: no schema structure or version, no metric extraction, no provenance compatibility rules, no exit codes, no comparator behaviour, no conversion behaviour, and no interaction with the #18 pressure handling (this branch is cut from sow/2026-Q3 after that merge, independent of it). Tests — 11 new offline cases driving the helpers with fabricated diagnostics (no OpenStudio install, no EnergyPlus run): successful run keeps a Warning; failed run keeps Warning and Error; benchmark-relevant Information becomes a note; routine Information does not; non-allow-listed codes still deliver their warnings; duplicates collapse; both arrays are ordinal-sorted; the same diagnostics in reversed order produce identical arrays; producer notes survive and are ordered with the rest; null/empty/whitespace inputs still serialize as valid empty arrays; and an integration-level mapping check that a SUCCESSFUL, schema-valid document now carries non-empty warnings and notes. Suites: producer 35 passed / 0 failed / 0 skipped (was 24). Full offline OpenStudio suite 331 passed / 0 failed / 2 skipped. C4 design-day subset 24 passed / 0 failed / 1 skipped. Generated by Michal Dengusiak and CodeClaude Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
… unavailable design-load metrics) (#20) * feat(results): read ZoneSizes into the result set and map per-space design loads wip checkpoint: core extraction + mapping + producer wiring build clean; offline tests and a regenerated benchmark artefact still to come. The OpenStudio route DOES produce per-space sizing loads — eplusout.sql has a structured ZoneSizes table (CalcDesLoad, UserDesLoad, CalcDesFlow, UserDesFlow, DesDayName, PeakHrMin, PeakTemp, PeakHumRat) — but nothing carried them into the SAM result graph that the benchmark reads, so every heating/cooling designLoad was reported unavailable: 28 of the 66 N/A metrics in the recorded cross-engine runs, the largest single coverage gap. Flow, deliberately through core rather than the CLI, so the benchmark keeps reporting the same SAM result graph every OpenStudio/Grasshopper consumer sees: eplusout.sql ZoneSizes -> OpenStudioSimulationRunner.ReadZoneSizes -> OpenStudioSimulationResultSet.ZoneSizing (new, deterministic ordinal order) -> Convert.ToSAM_SpaceDesignLoadResults -> SpaceSimulationResultParameter.DesignLoad -> existing benchmark mapping (unchanged) - New OpenStudioZoneSizingResult holds ALL eight columns for auditability; only UserDesLoad populates the v1 designLoad metric. UserDesLoad is the load after sizing factors, i.e. what EnergyPlus sizes components with, which is what the TAS route's TBD sizing load is too; CalcDesLoad stays on the result set for the coming design-day audit. - Sizing rows are matched on the EnergyPlus ThermalZone name (ZoneSizes.ZoneName), NOT the Ideal Loads key the annual dictionaries use — the sizing table is written by the zone sizing calculation, not a system output variable. - Heating and cooling map independently; a row without UserDesLoad yields nothing so a missing design load stays unavailable rather than becoming a zero, while a genuinely sized zero is emitted as an available zero. - peakLoad/peakHour are never written here: annual Load/LoadIndex remain the weather-run results, and a design load can never be relabelled as a simulated peak. - Unmatched EnergyPlus zones and duplicate (ZoneName, LoadType) rows both raise diagnostics (first row wins, explicitly, and the collision is named) instead of silent data loss. The producer feeds them through the diagnostic propagation merged in #19, so they surface in provenance.warnings. - ZoneSizing is an optional trailing constructor argument, so no existing result-set caller or fixture changes. Generated by Michal Dengusiak and CodeClaude Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test: pin zone-sizing design-load behaviour offline 16 offline tests (no OpenStudio install, no EnergyPlus run — the result set is fabricated). Core mapping (11, tests/…/C5/ZoneSizingDesignLoadTests): - UserDesLoad populates designLoad and CalcDesLoad explicitly does not; - heating and cooling map independently (heating-only sizing leaves cooling absent); - a genuinely sized zero is an available zero; - a row without UserDesLoad yields NO result, so the metric stays unavailable, never 0; - absent sizing yields no results and no diagnostics (not an anomaly); - the governing design day, peak outdoor temperature, peak time stamp and engine zone name are carried for audit; - Load/LoadIndex are never written by the sizing mapping; - an unmatched EnergyPlus zone raises a Warning naming the zone; - duplicate (ZoneName, LoadType) rows keep the first deterministically and report the collision; - the result set's zone-sizing order is independent of construction order; - null spaces yield an empty list. Benchmark mapping (5, benchmark/…/DesignLoadBenchmarkTests): - a sized zone reports designLoad from UserDesLoad in W, schema-valid; - adding sizing results does not disturb peakLoad/peakHour, which stay annual-run values; - an unsized load type stays unavailable with a null value; - a sized zero is reported as an available zero; - a run without sizing keeps the previous honest behaviour. Suites: producer 40 passed / 0 failed / 0 skipped (was 35). Full offline OpenStudio suite 342 passed / 0 failed / 2 skipped (was 331 / 0 / 2). Generated by Michal Dengusiak and CodeClaude Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: unify the design-load definition and make duplicate-row order total (Codex) Two P2 findings on 7f5629c, both correct. 1. Two consumers, two design loads. Create.SpaceSimulationResults — the established SQL path behind Modify.AddResults and the Grasshopper SQL component — wrote CalcDesLoad to SpaceSimulationResultParameter.DesignLoad, while the new converter writes UserDesLoad. A zone sizing factor would therefore make the same run report two different design loads, defeating the point of routing the benchmark through the shared result graph. That path now prefers UserDesLoad too, so there is ONE definition across consumers. CalcDesLoad remains the fallback only where the newer column does not exist: this reader also runs against older/synthetic ZoneSizes tables, and naming a missing column in the SELECT would fail the query and lose every design load. That is schema compatibility, not value policy — checked with PRAGMA table_info rather than guessed. A null design load is now also left absent instead of being written, so an unsized zone stays unavailable, never 0. 2. "First wins deterministically" was not true. Rows sharing a zone and load type share a Key, so the comparator returned 0 and List.Sort is NOT a stable sort; ReadZoneSizes also had no ORDER BY. Reversing the database row order could therefore change which UserDesLoad was emitted — exactly what the claim denied. Fixed with a real total order: OpenStudioZoneSizingResult carries SourceIndex (the SQL ZoneSizesIndex), the query selects and orders by ZoneName, LoadType, ZoneSizesIndex, and the result set orders by Key then SourceIndex. The tie-breaker's purpose is documented so it is not "tidied away" later. Tests: 4 new, 20 in total for this feature. - DuplicateRows_SurviveInputOrderReversalIdentically — the same rows in reversed order emit the same design load (this is the case the previous claim would have failed). - DuplicateRows_OrderInTheResultSetIsTotal — equal keys order by source index, independent of construction order. - EstablishedSqlConsumerPath_PrefersUserDesLoad — the legacy path emits UserDesLoad, matching the benchmark path, proven against a real on-disk SQL fixture. - EstablishedSqlConsumerPath_FallsBackWhenUserDesLoadColumnIsAbsent — the compatibility fallback still yields CalcDesLoad, so older tables keep working. The pre-existing AddResultsBySqlTests (whose synthetic ZoneSizes table has no UserDesLoad column) pass unchanged, which is the fallback working in situ. Suites: full offline OpenStudio 346 passed / 0 failed / 2 skipped (was 342/0/2); producer 40 passed / 0 failed / 0 skipped. Generated by Michal Dengusiak and CodeClaude Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: make zone-sizing order total on content, not just key and index (Codex) Codex found the same class of bug one level deeper on bc69c63, and was right. The tie-breaker I added only helps when callers supply a source index. A caller that does not — any fabricated row, and any future caller reading a table without ZoneSizesIndex — leaves duplicates sharing the DEFAULT index, so key and index both compare equal, the comparison reports equality, and List.Sort being unstable means reversing the input can still change which UserDesLoad is emitted. The documented guarantee was unconditional; the implementation was not. Ordering is now by OpenStudioZoneSizingResult.SortSignature — key, source index, then every payload field (both loads, both flows, design-day name, peak time, peak temperature, peak humidity ratio) in invariant round-trippable form. Two entries can therefore compare equal only when they are genuinely indistinguishable, in which case which one is chosen cannot change the answer. Ordering is total and independent of input order without needing the caller to cooperate. Tests: 2 new (17 for this feature, 20 including the benchmark-side ones). - DuplicateRows_WithoutSourceIndices_StillSurviveOrderReversalIdentically — duplicates with the DEFAULT index emit the same design load in either input order. This is the exact case the previous fix did not cover. - IndistinguishableRows_CompareEqualAndEitherIsTheSameAnswer — identical rows share a signature, which is correct rather than a gap. Suites: full offline OpenStudio 348 passed / 0 failed / 2 skipped; producer 40 passed / 0 failed / 0 skipped. Generated by Michal Dengusiak and CodeClaude Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(results): preserve result-set ABI and keep zone peak temperature correctly scoped Two review findings that survived to 372d052. A clean final review does not mean earlier inline findings were addressed, and neither of these was. 1. Binary compatibility was broken. Zone sizing was added as an OPTIONAL parameter on the existing public constructor, which preserves source compatibility but not binary: the 26-argument constructor no longer existed in metadata, so a Grasshopper or third-party assembly already compiled against it would throw MissingMethodException at run time. The original signature is now restored exactly and delegates to an explicit 27-argument overload; the optional default is gone, so the API has two real signatures. Pinned by reflection, because a 26-argument source call would bind just as happily to an optional parameter and prove nothing. Verified the test actually catches the regression: on the previous implementation it fails with "The original 26-argument constructor must remain in metadata for already-compiled consumers / Expected: not null / But was: null". It also invokes the preserved overload and asserts ZoneSizing comes back empty rather than null. 2. ZoneSizes.PeakTemp was written to DesignDayTemperature, which asserts a meaning the data does not settle. The EnergyPlus engineering reference describes the zone sizing peak temperature as a ZONE value (ZoneTempAtHeatPeak/ZoneTempAtCoolPeak). Against that, on the HungaryHouse run every ZoneSizes row carried -3.20000004768372 — bit-identical to that design day's OUTDOOR maximum dry bulb, where a heated zone would have sat at its setpoint instead. So the two readings disagree on real data. Either way the assignment had to go: writing an unsettled value into a parameter that names it an outdoor design-day temperature is precisely the silent relabelling this mapping exists to prevent, and the cost of removing it is nil because the value stays on OpenStudioZoneSizingResult. Its XML documentation now records both readings and the observed evidence instead of claiming outdoor dry bulb. NOT renamed to ZoneTemperatureAtPeak: that would bake in the zone reading, which is the one the observed data contradicts. The property keeps the SQL column's own name (PeakTemp -> PeakTemperature) and is documented as carried verbatim, pending the design-day audit. Tests: 19 for the core mapping (2 new — the reflection ABI test, and one proving a populated PeakTemp does not populate DesignDayTemperature while remaining readable on the result set). Suites: full offline OpenStudio 350 passed / 0 failed / 2 skipped; producer 40 passed / 0 failed / 0 skipped. Generated by Michal Dengusiak and CodeClaude Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(results): emit CalcDesLoad as designLoad, keeping sizing factors out of the metric Michal's call, reversing the earlier UserDesLoad decision once the evidence was in. Codex raised it as a P1 and was right on both counts: - docs/SAM_OPENSTUDIO_ANALYTICAL_COVERAGE.md already defines Result.Space.DesignLoad as "ZoneSizes CalcDesLoad (sizing runs)", so emitting UserDesLoad contradicted the repository's own result contract; and - the HungaryHouse run carries Sizing:Parameters 1.25 heating / 1.15 cooling, and every UserDesLoad was exactly CalcDesLoad x 1.25, so the published design loads carried a 25% margin. CalcDesLoad is the unaltered thermal load calculated from the design-day weather and schedules. UserDesLoad is the capacity after sizing factors — what components are sized with. A cross-engine benchmark must compare calculated loads, not fold a user-configured margin into an engine-physics question, so designLoad is now CalcDesLoad in BOTH consumers: - Convert.ToSAM_SpaceDesignLoadResults emits CalculatedDesignLoad, and a row without one yields no result (the metric stays unavailable rather than falling back to the other column); - Create.SpaceSimulationResults — Modify.AddResults and the Grasshopper SQL component — is returned to CalcDesLoad, so all consumers agree. The UserDesLoad column preference and its PRAGMA probe are removed; the DBNull guard is kept, since a null design load must stay absent rather than be written. UserDesLoad, both flows, the design day, peak time and PeakTemp/PeakHumRat all remain on OpenStudioZoneSizingResult for the sizing-capacity audit, and UserDesLoad may later be exposed as its own metric (sizingCapacity / userDesignLoad). Documentation on both types now states which value is the metric and why the other is not. Re-verified end to end on the real model. Coverage is unchanged — 92 of 148 comparable, 56 unavailable — because availability does not depend on which column is read. The values move materially, and in BOTH directions: e.g. 0872cd39 59.4% -> 49.3%, but 13a0403a 16.9% -> 33.5%. Worth recording, because it shows the choice is not cosmetic AND that TAS is not simply post-factor-consistent with OpenStudio: had both engines applied the same margin to comparable physics, removing it would have moved every row the same way. All 10 rows still Fail, which keeps pointing at the design-day translation mismatch rather than at sizing factors. Tests updated to the corrected definition, not merely renamed: CalcDesLoad is asserted as the emitted value with UserDesLoad asserted as still readable for audit; the missing-load case now withholds CalcDesLoad; and the legacy-path tests assert CalcDesLoad both when UserDesLoad is present and on tables without that column. Suites: full offline OpenStudio 350 passed / 0 failed / 2 skipped; producer 40 passed / 0 failed / 0 skipped. Generated by Michal Dengusiak and CodeClaude Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs: correct the design-load fallback sentence to CalcDesLoad The remarks on ToSAM_SpaceDesignLoadResults still said "A row with no UserDesLoad yields no result", left over from the reversed decision. The implementation checks CalculatedDesignLoad, so the sentence contradicted both the code and the surrounding documentation. It now names CalcDesLoad and states explicitly that a missing calculated load does NOT fall back to UserDesLoad — the metric stays unavailable. Documentation only; no behaviour change. Build clean, 19 design-load tests pass. 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>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
build: remove missed .NET Framework app.config leftover
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
…om upstream contribution Removes AGENTS.md, PROJECT_PROGRESS.md and internal evidence/.claude artefacts that are SAM-BIM workflow records, not upstream project documentation. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Drop the AI model/effort execution-model, implementation-brief and review-brief sections from the MVP plan (milestones table, smoke-test gate and architecture rules kept), neutralise stage/session/brief wording in the status, review and audit docs, and remove reviewer-name test comments. 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
Brings the SAM-BIM
sow/2026-Q3line of work intomaster(76 non-merge commits; 202 files changed, 47808 insertions(+), 231 deletions(-)).Main changes
(Full list in the commit history.)
Validation
Changes were merged through reviewed PRs on the SAM-BIM fork, with CI/builds run there; no further validation was run for this upstream PR.
Compatibility notes
Includes AGENTS.md / PROJECT_PROGRESS.md project-continuity files where present. Dependent SAM repos are being submitted in parallel from the same branch name; merge order should follow the SAM core dependency chain.
🤖 Generated with Claude Code