Skip to content

fix: handle Pydantic dict format in YAML numeric parser (GH#1235)#1236

Merged
sunt05 merged 5 commits intomasterfrom
sunt05/gh1235-fix-hdd-id-parser
Mar 10, 2026
Merged

fix: handle Pydantic dict format in YAML numeric parser (GH#1235)#1236
sunt05 merged 5 commits intomasterfrom
sunt05/gh1235-fix-hdd-id-parser

Conversation

@sunt05
Copy link

@sunt05 sunt05 commented Mar 10, 2026

Summary

  • Fix parse_numeric_sequence() in yaml_config.rs to handle Pydantic's named-key dict serialisation of hdd_id (e.g. {hdd_accum: 13.5, cdd_accum: 13.5, ...}), which was silently falling back to all-zero values
  • Regenerate sample_output.csv.gz with correct hdd_id values

Root cause

The Rust YAML parser only recognised the {value: [...]} wrapper for Mapping values. When the Python backend serialises config via Pydantic model_dump(), hdd_id becomes a named-key dict. The parser returned None, triggering a silent fallback to [0.0, ...] for heating/cooling degree-day accumulators. This affected anthropogenic heat flux (QF) and cascading variables.

The bug was introduced in #1209 (Rust bridge) but only became visible when #1116 regenerated the sample reference through the buggy path.

Verification

  • Pre-Vitor commit (77e861e37): applied parser fix only, both test_rust_backend_via_simulation and test_rust_bridge_sample_output pass against the known-good pre-Compute c2m/c2h dynamically from HF08 RSL theory #1116 reference
  • Current master + fix: both tests pass after reference regeneration
  • make test-smoke: 15 passed, 2 skipped

Test plan

  • CI passes test_rust_backend_via_simulation
  • Local test_rust_bridge_sample_output passes (CI skips — binary not at expected path)
  • make test-smoke passes

Fixes #1235

🤖 Generated with Claude Code

sunt05 and others added 2 commits March 10, 2026 15:33
`parse_numeric_sequence()` only accepted the `{value: [...]}` wrapper
for Mapping values. Pydantic serialises hdd_id as a named-key dict
(`{hdd_accum: 13.5, cdd_accum: 13.5, ...}`), causing silent fallback
to all-zero values for heating/cooling degree-day accumulators.

Add fallback that extracts numeric values from all mapping keys when
the standard RefValue wrapper is absent.

Fixes #1235

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The previous reference (from #1116) was generated through the buggy
parser path, baking incorrect hdd_id=0 into the baseline. Regenerate
using the fixed parser so the reference reflects correct anthropogenic
heat flux calculations.

Verified at pre-Vitor commit (77e861e) that both Python backend and
CLI paths produce output matching the known-good pre-#1116 reference.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions
Copy link

github-actions bot commented Mar 10, 2026

CI Build Plan

Changed Files

Rust bridge (1 file)

  • src/suews_bridge/src/yaml_config.rs

Tests (2 files)

  • test/core/test_sample_output.py
  • test/fixtures/data_test/sample_output.csv.gz

Build Configuration

Configuration
Platforms Linux x86_64, macOS ARM64, Windows x64
Python 3.9, 3.14
Test tier standard (all except slow)
UMEP build Yes (compiled extension may differ)
PR status Ready (standard matrix)

Rationale

  • Rust bridge changed -> multiplatform build required
  • Test files changed -> validation build
  • Compiled extension ABI may differ -> UMEP (NumPy 1.x) build included

Updated by CI on each push. See path-filters.yml for category definitions.

sunt05 and others added 3 commits March 10, 2026 15:40
The test only looked for the binary at the development path
(src/suews_bridge/target/release/suews), which doesn't exist in CI
where cibuildwheel installs the wheel. Fall back to the installed
binary via supy.cmd.rust_bridge._bridge_binary() so the test runs
in both development and CI environments.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Remove dead test_sample_output_validation (legacy run_supy API)
- Remove unused helpers: get_platform_info, save_debug_artifacts
- Remove dead imports: json, _DTS_AVAILABLE
- Rename test_rust_bridge_sample_output -> test_sample_output_validation
- Rename test_rust_backend_via_simulation -> test_library_cli_parity
- Trim parity test to 3 days (~5s) instead of full year (~18s)
- Trim stale module docstring

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Read Arrow output into bytes before closing the TemporaryDirectory
context. pyarrow.ipc.open_file keeps the file handle open, which
blocks cleanup on Windows (PermissionError WinError 32).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@sunt05 sunt05 added this pull request to the merge queue Mar 10, 2026
Merged via the queue into master with commit 09ecb38 Mar 10, 2026
16 checks passed
sunt05 added a commit that referenced this pull request Mar 10, 2026
Regenerated after rebasing onto master (includes #1231, #1236 fixes).
All physical bounds satisfied; SEB closes exactly at every timestep.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Test test_rust_bridge_sample_output failing in master branch

1 participant