Repository navigation
F-S11-1: associate OpenStudio zone results by name identity, not Guid suffix - #35
Merged
Merged
Conversation
… Guid suffix NormalizeKey matched ThermalZone/Space report keys to Ideal Loads keys by the last 8 characters and took the first hit, so rooms sharing a Guid prefix (s08-fix-two FixA/FixB, 5080c000) lost or overwrote unmet hours, gains, solar, infiltration and T/RH series. Match on <SanitizedName>_<GuidFirst8> after the SAM_<Type>_ prefix; absent/ambiguous identities and colliding report keys pass through unmapped, never guessed or overwritten. Offline synthetic-SQL tests. Generated by Michal Dengusiak and CodeClaude Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Generated by Michal Dengusiak and CodeClaude Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…atch Revision 1 stripped OpenStudio's " <n>" de-duplication ending and matched, so a numbered ThermalZone/Space key (possibly an unconditioned room with the same generated name) moved onto the sole unnumbered Ideal Loads key. Collect the identities of all numbered keys read by ExtractResultSet and leave numbered, colliding or numbered-target associations under their own keys. Adds scalar and series regressions: numbered source only, numbered target only, and a colliding unconditioned room whose quantity is the only source entry. Generated by Michal Dengusiak and CodeClaude Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…dence correction Generated by Michal Dengusiak and CodeClaude Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ventory Revision 2 detected collisions only from numbered keys of the result variables it read, so a colliding room without such output was invisible and its value could be assigned to the conditioned room. Read Zones.ZoneName (one row per EnergyPlus zone) and remap a key only when its identity is carried by exactly one unnumbered zone and by no numbered key read; with no usable inventory, nothing is remapped. Tests: hidden inventory-only collision (series on/off), distinct names without unmet output (series on/off), missing/empty/NULL/other inventory; existing tests now carry a native Zones inventory. Generated by Michal Dengusiak and CodeClaude Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Generated by Michal Dengusiak and CodeClaude Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
michaldengusiak
marked this pull request as ready for review
October 24, 2026 16:02
michaldengusiak
added a commit
that referenced
this pull request
Oct 24, 2026
…ature branch Integrates the merged F-S11-1 zone-result identity fix into Draft #34 before reconciling the S11 tests that pinned the defect. Generated by Michal Dengusiak and CodeClaude Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
michaldengusiak
added a commit
that referenced
this pull request
Oct 24, 2026
…he signed-off fixtures Replace the two tests that pinned the importer GUID-suffix remap (F-S11-1, fixed by #35) and SAM's integer UnmetHours (F-S11-2, fixed by SAM#219) with positive regressions through the production import, SAM-P/SAM-I and B1a: - SignedOffTwoZone_SharedGuidPrefix_EachSpaceKeepsItsOwnResults: unchanged s08-fix-two (FixA/FixB share 5080c000), zone-specific fractional unmet hours, distinct native/production identities, SAM-I attachment. - FractionalUnmetHours_SurviveSqlImport_SamSaveReload_AndB1a: 14.5/14.75 h through SQL import, sam.json and imported-model JSON save/reload, and B1a. Retire the re-GUIDed two-zone workaround: the S11 tests use the signed-off fixture. Synthetic fractional not-met time is now binary-exact quarter hours that differ per zone. No fixture, GUID, tolerance, metric or gate change. Generated by Michal Dengusiak and CodeClaude Co-Authored-By: Claude Opus 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.
Production fix for F-S11-1 only (found in S11, #34, which stays Draft and unchanged).
Defect:
OpenStudioSimulationRunner.NormalizeKeymapped ThermalZone/Space report keys to Ideal Loads keys by the last 8 characters and returned the first hit. Rooms sharing a Guid prefix (signed-offs08-fix-twoFixA/FixB,5080c000) lost or overwrote each other's unmet hours, gains, window solar, infiltration and T/T_op/RH series.Fix (one shared helper): match on the generated-name identity
<SanitizedName>_<GuidFirst8>after a knownSAM_IdealLoads_/SAM_ThermalZone_/SAM_Space_prefix, case-insensitive. A key is remapped only when its identity is proven unique by the native SQL zone inventory (Zones.ZoneName: exactly one unnumbered zone carries it) and no key read carries it numbered, and exactly one unnumbered Ideal Loads key matches. Otherwise, including when the inventory is missing, empty or lacks the room, the key stays under its own name — never guessed, never overwritten. Writer names, Guid formats, schemas and public APIs unchanged.Revisions:
<n>de-duplication ending is collision evidence only, never a match; record evidence wording corrected (synthetic review ESO vs genuine EnergyPlus 25.2.0 output).extractTimeSeries; it uses the SQL Zones inventory, with report keys as a supplement.Tests:
C5/ZoneResultIdentityTests.cs, 17 offline synthetic-SQL cases with a native Zones inventory. The r3 cases (hidden inventory-only collision with series on/off; missing/empty/NULL/other inventory) failed 6/17 on r2 code48668820and pass now; distinct-name/shared-prefix cases pass on both. Fresh Release x64 rebuild 0 errors; full offline suite (TestCategory!=Simulation) 488 passed / 0 failed / 2 skipped. The reviewer's PowerShell 7 probe was not rerun (no PowerShell 7 on this machine); its scenario is covered by theHiddenCollision_…tests. No engine run, no Grasshopper deployment.Tested content:
b8b38e6ed4a622ffacd914a22b027ad245d3af7b; head54026386is record-only. Earlier: r2 content48668820/ head77dd095b, r1 content13e79b04/ heada25b4f5a(Build/SPDX green on both). Hosted CI builds the solution and runs the S09 tests; it does not run these main-project tests. Record:documentation/F-S11-1-Zone-Result-Identity-PR.md.Out of scope: F-S11-2 fractional unmet hours, OccupiedUnmetHours, AddResults suffix lookups (can collide even for different names sharing a prefix; separate scope), fixtures, S11 tolerances, writer naming, UI, CI.
Next: independent review of revision 3. Not Ready, do not merge.
Generated by Michal Dengusiak and CodeClaude
🤖 Generated with Claude Code