From 3902dd2418db0e152411bc4197ffab2c71fe37c2 Mon Sep 17 00:00:00 2001 From: Amir Hajdar Date: Tue, 6 Oct 2026 20:46:20 +0200 Subject: [PATCH 1/2] AgentiGrid MATPOWER reader/writer: reject malformed input, stable round trips Five defects found while specifying the parser and writer: 1. Rows with a non-numeric value were skipped with only a log warning, so a bus, generator or branch could silently disappear; too-short rows crashed with IndexError. Known sections (bus, gen, branch, gencost) now raise ValueError naming the section and row for non-numeric values, column counts outside the section's limits, and non-integral values in integer columns. 2. Valid MATLAB syntax was misread: two rows on one line became one garbage row and comma-separated rows were all dropped. These, '...' continuations, unterminated blocks and a missing mpc.bus now raise ValueError. 3. The writer crashed on Inf/NaN, which the reader accepts. They are written as Inf/-Inf/NaN; bus solution columns no longer lose precision to .4f. 4. Every read/write round trip added another "MATPOWER Case Format" header line. Writing is now idempotent. 5. Case version '1' was accepted although ExaGO reads version 2; it now raises. A missing version still means '2'. CRLF input is normalised on read and output is always LF. The function line always carries network.casename. All 29 case files in datafiles/ still parse and round-trip to equivalent networks. New tests/grid_files/test_matpower_hardening.py (29 tests, 22 fail without the fix). Co-Authored-By: Claude Opus 5.5 --- .../exago_parsers/matpower_parser.py | 132 +++++++--- .../exago_parsers/matpower_writer.py | 39 ++- agentigrid/claude_code_changes.md | 22 ++ .../grid_files/test_matpower_hardening.py | 230 ++++++++++++++++++ 4 files changed, 382 insertions(+), 41 deletions(-) create mode 100644 agentigrid/tests/grid_files/test_matpower_hardening.py diff --git a/agentigrid/agentigrid/exago_parsers/matpower_parser.py b/agentigrid/agentigrid/exago_parsers/matpower_parser.py index 908bda1e..34d0dffd 100644 --- a/agentigrid/agentigrid/exago_parsers/matpower_parser.py +++ b/agentigrid/agentigrid/exago_parsers/matpower_parser.py @@ -3,6 +3,7 @@ from __future__ import annotations import logging +import math import re from pathlib import Path @@ -26,6 +27,25 @@ # Match mpc.
= [ or mpc.
= { _SECTION_RE = re.compile(r"^mpc\.(\w+)\s*=\s*([{\[])", re.MULTILINE) +# Only MATPOWER case format version 2 is supported (ExaGO reads version 2). +_SUPPORTED_VERSIONS = ("2",) + +# Sections parsed into dataclasses; any other section is kept as raw text. +_KNOWN_SECTIONS = ("bus", "gen", "branch", "gencost") +# Column limits per known section. Bus rows have 13 input columns plus up to +# 4 optional solution columns (lam_P, lam_Q, mu_Vmax, mu_Vmin); gen and +# branch rows may carry any number of extra columns, which are kept. +_MIN_COLS = {"bus": 13, "gen": 10, "branch": 13, "gencost": 4} +_MAX_COLS = {"bus": 17} +# 0-based columns that must hold integers (MATPOWER BUS_I, BUS_TYPE, +# BUS_AREA, ZONE; GEN_BUS, GEN_STATUS; F_BUS, T_BUS, BR_STATUS; MODEL, NCOST). +_INT_COLS = { + "bus": (0, 1, 6, 10), + "gen": (0, 7), + "branch": (0, 1, 10), + "gencost": (0, 3), +} + def _parse_float(s: str) -> float: """Parse a string as float, handling MATLAB Inf/NaN.""" @@ -39,21 +59,61 @@ def _parse_float(s: str) -> float: return float(sl) -def _split_data_row(line: str) -> list[str]: - """Split a MATPOWER data row into tokens, stripping comments and semicolons.""" - # Remove inline comment - idx = line.find("%") - if idx >= 0: - line = line[:idx] - # Remove trailing semicolons and whitespace - line = line.rstrip().rstrip(";").rstrip() - return line.split() - +def _parse_rows(section: str, block: str) -> list[list[float]]: + """Parse the rows of a known numeric section (one row per line). -def _extract_section_block(text: str, start: int, bracket: str) -> tuple[str, int]: + Anything that would otherwise be misread raises ``ValueError`` naming the + section and the 1-based data row: unsupported syntax (several rows on one + line, commas, ``...`` continuations), non-numeric values, a column count + outside the section's limits, or a non-integral value in an integer column. + Rows are never skipped silently. + """ + rows: list[list[float]] = [] + for line in block.split("\n"): + idx = line.find("%") + if idx >= 0: + line = line[:idx] + line = line.strip().rstrip(";").rstrip() + if not line: + continue + where = f"mpc.{section} row {len(rows) + 1}" + if "..." in line: + raise ValueError(f"{where}: line continuation '...' is not supported") + if ";" in line: + raise ValueError( + f"{where}: several rows on one line are not supported; " + "put each row on its own line" + ) + if "," in line: + raise ValueError( + f"{where}: comma-separated values are not supported; " + "separate values with spaces or tabs" + ) + try: + values = [_parse_float(t) for t in line.split()] + except ValueError: + raise ValueError(f"{where}: non-numeric value in {line!r}") from None + lo, hi = _MIN_COLS[section], _MAX_COLS.get(section) + if len(values) < lo or (hi is not None and len(values) > hi): + expected = f"{lo}-{hi}" if hi is not None else f"at least {lo}" + raise ValueError(f"{where}: {len(values)} columns, expected {expected}") + for col in _INT_COLS[section]: + v = values[col] + if not math.isfinite(v) or v != int(v): + raise ValueError(f"{where}: column {col + 1} must be an integer, got {v!r}") + rows.append(values) + return rows + + +def _extract_section_block( + text: str, start: int, bracket: str, section: str = "?" +) -> tuple[str, int]: """Extract text from *start* until the matching closing bracket. Returns (block_content, end_position). + + Raises: + ValueError: If the block is never closed. """ close = "]" if bracket == "[" else "}" # Find the end of the section @@ -66,6 +126,8 @@ def _extract_section_block(text: str, start: int, bracket: str) -> tuple[str, in elif ch == close: depth -= 1 i += 1 + if depth > 0: + raise ValueError(f"mpc.{section}: '{bracket}' block is not terminated") return text[start:i - 1], i @@ -80,6 +142,11 @@ def _collect_section_comment(text: str, section_start: int) -> str: else: break comment_lines.reverse() + # Drop surrounding blank lines so a read/write round trip is stable. + while comment_lines and not comment_lines[0].strip(): + comment_lines.pop(0) + while comment_lines and not comment_lines[-1].strip(): + comment_lines.pop() return "\n".join(comment_lines) @@ -180,7 +247,8 @@ def parse_matpower(path: Path) -> MATNetwork: if not path.exists(): raise FileNotFoundError(f"MATPOWER file not found: {path}") - text = path.read_text(encoding="utf-8") + # Normalise line endings: CRLF (Windows-authored) files read the same as LF. + text = path.read_text(encoding="utf-8").replace("\r\n", "\n") # --- Function name --- m = _FUNC_RE.search(text) @@ -191,6 +259,11 @@ def parse_matpower(path: Path) -> MATNetwork: # --- Version --- m = _VERSION_RE.search(text) version = m.group(1) if m else "2" + if version not in _SUPPORTED_VERSIONS: + raise ValueError( + f"Unsupported MATPOWER case version '{version}' in {path}; " + f"supported: {', '.join(_SUPPORTED_VERSIONS)}" + ) # --- baseMVA --- m = _BASEMVA_RE.search(text) @@ -222,12 +295,13 @@ def parse_matpower(path: Path) -> MATNetwork: if section_name in ("version", "baseMVA"): continue - block_content, _ = _extract_section_block(text, block_start, bracket) + block_content, _ = _extract_section_block(text, block_start, bracket, section_name) # Collect preceding comment line_start = text.rfind("\n", 0, match.start()) + 1 comment = _collect_section_comment(text, line_start) _section_comments[section_name] = comment + prefix = comment + "\n" if comment else "" if bracket == "{": # Cell array (bus_name, gentype, genfuel, etc.) → store as raw @@ -235,23 +309,19 @@ def parse_matpower(path: Path) -> MATNetwork: # mpc.genfuel by searching for "};", so a bare "}" makes it skip the # fuel types (and with them the default ramp rates and the # renewable Pmin=0 relaxation). - raw_text = comment + "\n" + text[match.start():block_start] + block_content + "};" + raw_text = prefix + text[match.start():block_start] + block_content + "};" + extra_sections[section_name] = raw_text + continue + + if section_name not in _KNOWN_SECTIONS: + # Unknown numeric section (areas, dcline, ...) → store as raw + raw_text = prefix + text[match.start():block_start] + block_content + "];" extra_sections[section_name] = raw_text + logger.debug("Stored unknown section mpc.%s as raw text", section_name) continue - # Numeric matrix section — parse rows - rows: list[list[float]] = [] - for line in block_content.split("\n"): - stripped = line.strip() - if not stripped or stripped.startswith("%"): - continue - tokens = _split_data_row(line) - if not tokens: - continue - try: - rows.append([_parse_float(t) for t in tokens]) - except ValueError: - logger.warning("Skipping unparseable row in mpc.%s: %s", section_name, line.strip()) + # Known numeric matrix section — parse rows (raises on bad input) + rows = _parse_rows(section_name, block_content) if section_name == "bus": buses = [_row_to_bus(r) for r in rows] @@ -265,11 +335,9 @@ def parse_matpower(path: Path) -> MATNetwork: elif section_name == "gencost": gencost = [_row_to_gencost(r) for r in rows] logger.debug("Parsed %d gencost entries", len(gencost)) - else: - # Unknown numeric section → store as raw - raw_text = comment + "\n" + text[match.start():block_start] + block_content + "];" - extra_sections[section_name] = raw_text - logger.debug("Stored unknown section mpc.%s as raw text", section_name) + + if "bus" not in _section_comments: + raise ValueError(f"Cannot find mpc.bus in {path}") # Store section comments on the network for the writer net = MATNetwork( diff --git a/agentigrid/agentigrid/exago_parsers/matpower_writer.py b/agentigrid/agentigrid/exago_parsers/matpower_writer.py index 7ff2898d..9ae60996 100644 --- a/agentigrid/agentigrid/exago_parsers/matpower_writer.py +++ b/agentigrid/agentigrid/exago_parsers/matpower_writer.py @@ -3,6 +3,8 @@ from __future__ import annotations import logging +import math +import re from pathlib import Path from agentigrid.exago_parsers.matpower_model import ( @@ -15,9 +17,19 @@ logger = logging.getLogger("agentigrid.exago_parsers.matpower") +_FUNC_RE = re.compile(r"^function\s+mpc\s*=\s*\w+", re.MULTILINE) + def _fmt(value: float) -> str: - """Format a numeric value, writing integers without decimals.""" + """Format a numeric value, writing integers without decimals. + + Infinities and NaN are written as MATLAB ``Inf`` / ``-Inf`` / ``NaN`` + (the parser reads them back). + """ + if math.isnan(value): + return "NaN" + if math.isinf(value): + return "Inf" if value > 0 else "-Inf" if value == int(value) and abs(value) < 1e15: return str(int(value)) return f"{value:.10g}" @@ -31,7 +43,7 @@ def _write_bus_row(b: Bus) -> str: ] for val in (b.lam_P, b.lam_Q, b.mu_Vmax, b.mu_Vmin): if val is not None: - parts.append(f"{val:.4f}") + parts.append(_fmt(val)) return "\t" + "\t".join(parts) + ";" @@ -93,14 +105,22 @@ def write_matpower(network: MATNetwork, path: Path) -> None: lines: list[str] = [] # --- Header --- - lines.append(network.header_comments.rstrip("\n")) - # Ensure function line is present (header_comments includes it) - if f"function mpc = {network.casename}" not in network.header_comments: - lines.append(f"function mpc = {network.casename}") + # The function line always carries network.casename: an existing one in + # header_comments is renamed, a missing one is added as the first line. + header = network.header_comments.rstrip("\n") + func_line = f"function mpc = {network.casename}" + if _FUNC_RE.search(header): + header = _FUNC_RE.sub(lambda _m: func_line, header, count=1) + else: + header = func_line + ("\n" + header if header else "") + lines.append(header) # --- Version & baseMVA --- - lines.append("") - lines.append(f"%% MATPOWER Case Format : Version {network.version}") + # The header read back from a file already holds the format comment; + # adding it again would grow the file on every read/write round trip. + if "MATPOWER Case Format" not in header: + lines.append("") + lines.append(f"%% MATPOWER Case Format : Version {network.version}") lines.append(f"mpc.version = '{network.version}';") lines.append("") lines.append("%%----- Power Flow Data -----%%") @@ -154,5 +174,6 @@ def write_matpower(network: MATNetwork, path: Path) -> None: lines.append("") - path.write_text("\n".join(lines) + "\n", encoding="utf-8") + # Always LF line endings, whatever the platform or the source file used. + path.write_text("\n".join(lines) + "\n", encoding="utf-8", newline="\n") logger.info("Wrote MATPOWER file: %s", path) diff --git a/agentigrid/claude_code_changes.md b/agentigrid/claude_code_changes.md index d673d256..17433ecc 100644 --- a/agentigrid/claude_code_changes.md +++ b/agentigrid/claude_code_changes.md @@ -4,6 +4,28 @@ This document records significant changes made by Claude Code, grouped by the pr --- +## MATPOWER reader/writer hardening — no silent misreads, stable round trips (2026-10-05) + +Found while writing an independent spec for the parser and writer (round-trip, error and syntax rules). Every real case in ExaGO's `datafiles/` (29 case files, `case5` to `case_ACTIVSg10k`, including CRLF ones) still parses and round-trips to an equivalent network; the files are already accepted by the stricter rules. + +| # | Defect | Fix | +|---|---|---| +| 1 | A row with a non-numeric value was skipped with only a log warning, so a bus, generator or branch could silently vanish. Too-short rows crashed with `IndexError`. | `_parse_rows()` raises `ValueError` naming the section and 1-based row for non-numeric values, column counts outside the section's limits (bus 13–17; gen ≥ 10; branch ≥ 13; gencost ≥ 4) and non-integral/Inf/NaN values in integer columns. | +| 2 | Valid MATLAB syntax was misread: two rows on one line became one garbage row; comma-separated rows were all dropped; `...` continuations crashed. | These raise `ValueError` with a clear message. An unterminated `[`/`{` block and a missing `mpc.bus` raise as well. | +| 3 | The writer crashed on `Inf`/`NaN` (`OverflowError`/`ValueError`), which the reader accepts. | Written as `Inf`, `-Inf`, `NaN`. Bus solution columns (`lam_*`, `mu_*`) use the same 10-significant-digit format instead of `.4f`. | +| 4 | Every read/write round trip added another `%% MATPOWER Case Format` line, and blank lines accumulated before raw sections. | The format comment is written only if the header lacks it; collected section comments drop surrounding blank lines. Writing is idempotent. | +| 5 | MATPOWER case version `'1'` was accepted although ExaGO reads version 2. | Raises `ValueError`; a missing version still means `'2'`. | + +Also: CRLF input is normalised on read and the writer always emits LF; the `function mpc = …` line always carries `network.casename` (renamed if the header has another name, added first if missing). Unknown numeric sections (`areas`, `dcline`, …) stay opaque raw text and are not validated. + +| File | Change | +|---|---| +| `agentigrid/exago_parsers/matpower_parser.py` | Fixes 1, 2, 4 (comments), 5; CRLF normalisation | +| `agentigrid/exago_parsers/matpower_writer.py` | Fixes 3, 4 (header); casename/function line; LF output | +| `tests/grid_files/test_matpower_hardening.py` | New (29 tests; 22 fail without the fix) | + +--- + ## MATPOWER cell arrays keep "};" — ExaGO reads genfuel again (2026-10-03) **Symptom.** SCOPFLOW reported local infeasibility on AgentiGrid-rewritten ACTIVSg200 at base load, while the original file converged. The rewritten file had `mpc.genfuel = { ... }` with a bare `}`. diff --git a/agentigrid/tests/grid_files/test_matpower_hardening.py b/agentigrid/tests/grid_files/test_matpower_hardening.py new file mode 100644 index 00000000..98c0d857 --- /dev/null +++ b/agentigrid/tests/grid_files/test_matpower_hardening.py @@ -0,0 +1,230 @@ +"""MATPOWER reader/writer hardening: no silent misreads, stable round trips. + +Covers five defects found while specifying the parser: + 1. malformed rows were skipped with only a log warning (a bus could vanish); + 2. valid-but-unsupported MATLAB syntax was misread (two rows on one line + became one garbage row; comma-separated rows were all dropped); + 3. the writer crashed on Inf/NaN, which the reader accepts; + 4. every read/write round trip added another header line; + 5. MATPOWER case version '1' was accepted although ExaGO reads version 2. +""" + +from __future__ import annotations + +import copy +import math +from pathlib import Path + +import pytest + +from agentigrid.exago_parsers.matpower_model import Branch, Bus, GenCost, Generator, MATNetwork +from agentigrid.exago_parsers.matpower_parser import parse_matpower +from agentigrid.exago_parsers.matpower_writer import write_matpower + +ROOT = Path(__file__).resolve().parents[2] +ACTIVSG200 = ROOT.parent / "datafiles" / "case_ACTIVSg200.m" + +BUS = "\t1\t3\t0\t0\t0\t0\t1\t1\t0\t230\t1\t1.1\t0.9;\n\t2\t1\t90\t30\t0\t0\t1\t1\t0\t230\t1\t1.1\t0.9;" +GEN = "\t1\t0\t0\t300\t-300\t1\t100\t1\t250\t10;" +BRANCH = "\t1\t2\t0.01\t0.1\t0\t100\t100\t100\t0\t0\t1\t-360\t360;" +GENCOST = "\t2\t0\t0\t3\t0.11\t5\t150;" + + +def _case(*, version="2", bus=BUS, gen=GEN, branch=BRANCH, gencost=GENCOST, extra="", + drop: tuple[str, ...] = ()) -> str: + parts = { + "head": "function mpc = tiny\n%TINY test case\n\n%% MATPOWER Case Format : Version 2\n", + "version": f"mpc.version = '{version}';\n", + "base": "\n%% system MVA base\nmpc.baseMVA = 100;\n", + "bus": f"\n%% bus data\nmpc.bus = [\n{bus}\n];\n", + "gen": f"\n%% generator data\nmpc.gen = [\n{gen}\n];\n", + "branch": f"\n%% branch data\nmpc.branch = [\n{branch}\n];\n", + "gencost": f"\n%% generator cost data\nmpc.gencost = [\n{gencost}\n];\n", + "extra": f"\n{extra}\n" if extra else "", + } + return "".join(v for k, v in parts.items() if k not in drop) + + +def _write(tmp_path: Path, text: str, name: str = "c.m", crlf: bool = False) -> Path: + p = tmp_path / name + data = text.replace("\n", "\r\n") if crlf else text + p.write_bytes(data.encode("utf-8")) + return p + + +# --- 5. version ------------------------------------------------------------ + +def test_version_1_rejected(tmp_path): + with pytest.raises(ValueError, match="version"): + parse_matpower(_write(tmp_path, _case(version="1"))) + + +def test_missing_version_defaults_to_2(tmp_path): + text = _case().replace("mpc.version = '2';\n", "") + assert parse_matpower(_write(tmp_path, text)).version == "2" + + +# --- 1./2. malformed rows and unsupported syntax --------------------------- + +@pytest.mark.parametrize( + "kwargs, section", + [ + ({"bus": BUS.replace("\n", " ")}, "bus"), # two rows, one line + ({"bus": BUS.replace("\t", ",")}, "bus"), # commas + ({"gen": "\t1\t0\t0\t300\t-300 ...\n\t1\t100\t1\t250\t10;"}, "gen"), # continuation + ({"bus": BUS + "\n\t3\tX\t0\t0\t0\t0\t1\t1\t0\t230\t1\t1.1\t0.9;"}, "bus"), # non-numeric + ({"bus": "\t1\t3\t0\t0;"}, "bus"), # too few columns + ({"gen": "\t1\t0\t0;"}, "gen"), + ({"branch": "\t1\t2\t0.01;"}, "branch"), + ({"gencost": "\t2\t0\t0;"}, "gencost"), + ({"bus": BUS + "\n\t3" + "\t1" * 17 + ";"}, "bus"), # 18 bus columns + ({"bus": BUS.replace("\t1\t3\t", "\t1.5\t3\t", 1)}, "bus"), # non-integral bus_i + ({"gen": GEN.replace("\t100\t1\t", "\t100\tInf\t")}, "gen"), # Inf status + ({"branch": BRANCH.replace("\t0\t1\t-360", "\t0\tNaN\t-360")}, "branch"), # NaN status + ], + ids=["rows-one-line", "commas", "continuation", "non-numeric", "short-bus", "short-gen", + "short-branch", "short-gencost", "bus-18-cols", "int-col-1.5", "int-col-inf", + "int-col-nan"], +) +def test_malformed_input_raises_naming_section(tmp_path, kwargs, section): + with pytest.raises(ValueError, match=rf"mpc\.{section}\b"): + parse_matpower(_write(tmp_path, _case(**kwargs))) + + +def test_non_numeric_row_reports_row_number(tmp_path): + bad = BUS + "\n\t3\tX\t0\t0\t0\t0\t1\t1\t0\t230\t1\t1.1\t0.9;" + with pytest.raises(ValueError, match=r"mpc\.bus row 3"): + parse_matpower(_write(tmp_path, _case(bus=bad))) + + +def test_unterminated_block_raises(tmp_path): + text = _case(drop=("gencost",)).replace("\n];\n\n%% branch data", "\n\n%% branch data") + with pytest.raises(ValueError, match="terminated"): + parse_matpower(_write(tmp_path, text)) + + +def test_missing_bus_section_raises(tmp_path): + with pytest.raises(ValueError, match="mpc.bus"): + parse_matpower(_write(tmp_path, _case(drop=("bus",)))) + + +def test_missing_gen_and_branch_give_empty_lists(tmp_path): + net = parse_matpower(_write(tmp_path, _case(drop=("gen", "branch", "gencost")))) + assert net.generators == [] and net.branches == [] and net.gencost == [] + + +def test_partial_bus_solution_columns(tmp_path): + net = parse_matpower(_write(tmp_path, _case(bus=BUS.replace("0.9;", "0.9\t6.5;", 1)))) + b = net.buses[0] + assert b.lam_P == 6.5 and b.lam_Q is None and b.mu_Vmax is None and b.mu_Vmin is None + + +def test_integral_floats_accepted_in_integer_columns(tmp_path): + net = parse_matpower(_write(tmp_path, _case(bus=BUS.replace("\t1\t3\t", "\t1.0\t3.0\t", 1)))) + assert net.buses[0].bus_i == 1 and isinstance(net.buses[0].bus_i, int) + + +# --- line endings ------------------------------------------------------------ + +def test_crlf_reads_like_lf_and_writer_emits_lf(tmp_path): + lf = parse_matpower(_write(tmp_path, _case(), "lf.m")) + crlf = parse_matpower(_write(tmp_path, _case(), "crlf.m", crlf=True)) + assert crlf.buses == lf.buses and crlf.header_comments == lf.header_comments + out = tmp_path / "out.m" + write_matpower(crlf, out) + assert b"\r" not in out.read_bytes() + + +# --- 3. Inf / NaN ---------------------------------------------------------- + +def test_inf_nan_round_trip(tmp_path): + net = parse_matpower(_write(tmp_path, _case())) + net.branches[0].rateA = math.inf + net.generators[0].Qmin = -math.inf + net.buses[1].Pd = math.nan + out = tmp_path / "out.m" + write_matpower(net, out) + back = parse_matpower(out) + assert back.branches[0].rateA == math.inf + assert back.generators[0].Qmin == -math.inf + assert math.isnan(back.buses[1].Pd) + + +def test_solution_columns_keep_precision(tmp_path): + net = parse_matpower(_write(tmp_path, _case(bus=BUS.replace("0.9;", "0.9\t6.5\t0\t1e-07\t0.000123456789;", 1)))) + out = tmp_path / "out.m" + write_matpower(net, out) + b = parse_matpower(out).buses[0] + assert math.isclose(b.mu_Vmax, 1e-07, rel_tol=1e-9) + assert math.isclose(b.mu_Vmin, 0.000123456789, rel_tol=1e-9) + + +# --- 4. idempotent writing --------------------------------------------------- + +EXTRAS = ( + "%% generator fuel type\nmpc.genfuel = {\n\t'coal';\n};\n\n" + "mpc.areas = [\n\t1\t1;\n];" +) + + +def test_round_trip_text_is_idempotent(tmp_path): + src = _write(tmp_path, _case(extra=EXTRAS), crlf=True) + a, b, c = (tmp_path / n for n in ("a.m", "b.m", "c.m")) + write_matpower(parse_matpower(src), a) + write_matpower(parse_matpower(a), b) + write_matpower(parse_matpower(b), c) + assert a.read_text() == b.read_text() == c.read_text() + assert a.read_text().count("MATPOWER Case Format") == 1 + + +@pytest.mark.skipif(not ACTIVSG200.exists(), reason="ExaGO datafiles not present") +def test_real_case_round_trip_is_idempotent(tmp_path): + a, b = tmp_path / "a.m", tmp_path / "b.m" + write_matpower(parse_matpower(ACTIVSG200), a) + write_matpower(parse_matpower(a), b) + assert a.read_text() == b.read_text() + + +def test_section_comments_stay_with_their_section(tmp_path): + out = tmp_path / "out.m" + write_matpower(parse_matpower(_write(tmp_path, _case(extra=EXTRAS))), out) + text = out.read_text() + assert text.index("%% generator data") < text.index("mpc.gen =") + assert text.index("];", text.index("mpc.bus =")) < text.index("%% generator data") + assert text.index("%% generator fuel type") < text.index("mpc.genfuel =") + + +# --- casename / function line ------------------------------------------------ + +def _net(header: str, casename: str = "mine") -> MATNetwork: + return MATNetwork( + casename=casename, version="2", baseMVA=100.0, + buses=[Bus(1, 3, 0.0, 0.0, 0.0, 0.0, 1, 1.0, 0.0, 230.0, 1, 1.1, 0.9)], + generators=[Generator(1, 0.0, 0.0, 300.0, -300.0, 1.0, 100.0, 1, 250.0, 10.0)], + branches=[Branch(1, 1, 0.01, 0.1, 0.0, 100.0, 100.0, 100.0, 0.0, 0.0, 1, -360.0, 360.0)], + gencost=[GenCost(2, 0.0, 0.0, 3, [0.11, 5.0, 150.0])], + header_comments=header, + ) + + +def test_casename_wins_over_header_function_line(tmp_path): + out = tmp_path / "out.m" + write_matpower(_net("function mpc = other\n%comment\n", casename="mine"), out) + text = out.read_text() + assert "function mpc = mine" in text and "function mpc = other" not in text + assert text.count("function mpc") == 1 + assert parse_matpower(out).casename == "mine" + + +def test_empty_header_writes_function_line_first(tmp_path): + out = tmp_path / "out.m" + write_matpower(_net(""), out) + assert out.read_text().splitlines()[0] == "function mpc = mine" + assert parse_matpower(out).casename == "mine" + + +def test_writer_does_not_mutate_its_input(tmp_path): + net = parse_matpower(_write(tmp_path, _case(extra=EXTRAS))) + before = copy.deepcopy(net) + write_matpower(net, tmp_path / "out.m") + assert net == before From 9db9ac71e49574aa2532d9f4b7b939fbea0e2950 Mon Sep 17 00:00:00 2001 From: Amir Hajdar Date: Tue, 6 Oct 2026 20:46:20 +0200 Subject: [PATCH 2/2] AgentiGrid MATPOWER writer/reader: exact near-max floats, idempotent rounding, baseMVA literals Three defects found by property-based round-trip testing: 1. A finite value near the float maximum was written with 10 significant digits that round past it and read back as Inf. The writer now falls back to the exact representation when rounding overflows. 2. A value that rounds to an integer (e.g. 9999999999.5) was written as "1e+10" the first time and "10000000000" after re-reading, so writing was not idempotent. The integer form is now decided on the rounded value. 3. mpc.baseMVA was read with a digits-and-dots pattern: "1e2" (and "1e+15", which the writer itself emits for large values) was silently read as 1, and a negative value was reported as missing. Any numeric literal (sign, exponent, Inf/NaN) on an mpc.baseMVA line is now accepted, text in comments is ignored, and a non-numeric value raises ValueError. 18 more tests in tests/grid_files/test_matpower_hardening.py (12 fail without the fix). All 29 case files in datafiles/ still round-trip. Co-Authored-By: Claude Opus 5.5 --- .../exago_parsers/matpower_parser.py | 9 +++- .../exago_parsers/matpower_writer.py | 14 ++++-- agentigrid/claude_code_changes.md | 8 +++ .../grid_files/test_matpower_hardening.py | 49 +++++++++++++++++++ 4 files changed, 75 insertions(+), 5 deletions(-) diff --git a/agentigrid/agentigrid/exago_parsers/matpower_parser.py b/agentigrid/agentigrid/exago_parsers/matpower_parser.py index 34d0dffd..6520512e 100644 --- a/agentigrid/agentigrid/exago_parsers/matpower_parser.py +++ b/agentigrid/agentigrid/exago_parsers/matpower_parser.py @@ -23,7 +23,8 @@ _FUNC_RE = re.compile(r"^function\s+mpc\s*=\s*(\w+)", re.MULTILINE) _VERSION_RE = re.compile(r"mpc\.version\s*=\s*'([^']+)'") -_BASEMVA_RE = re.compile(r"mpc\.baseMVA\s*=\s*([\d.]+)") +# The value is any numeric token (sign, exponent, Inf/NaN) up to ";" or a comment. +_BASEMVA_RE = re.compile(r"^[ \t]*mpc\.baseMVA[ \t]*=[ \t]*([^;%\n]*)", re.MULTILINE) # Match mpc.
= [ or mpc.
= { _SECTION_RE = re.compile(r"^mpc\.(\w+)\s*=\s*([{\[])", re.MULTILINE) @@ -269,7 +270,11 @@ def parse_matpower(path: Path) -> MATNetwork: m = _BASEMVA_RE.search(text) if not m: raise ValueError(f"Cannot find mpc.baseMVA in {path}") - baseMVA = float(m.group(1)) + token = m.group(1).strip() + try: + baseMVA = _parse_float(token) + except ValueError: + raise ValueError(f"mpc.baseMVA: invalid value {token!r} in {path}") from None # --- Header comments (everything before mpc.version or mpc.baseMVA) --- first_mpc = text.find("mpc.") diff --git a/agentigrid/agentigrid/exago_parsers/matpower_writer.py b/agentigrid/agentigrid/exago_parsers/matpower_writer.py index 9ae60996..71db52ff 100644 --- a/agentigrid/agentigrid/exago_parsers/matpower_writer.py +++ b/agentigrid/agentigrid/exago_parsers/matpower_writer.py @@ -30,9 +30,17 @@ def _fmt(value: float) -> str: return "NaN" if math.isinf(value): return "Inf" if value > 0 else "-Inf" - if value == int(value) and abs(value) < 1e15: - return str(int(value)) - return f"{value:.10g}" + text = f"{value:.10g}" + rounded = float(text) + if math.isinf(rounded): + # Rounding to 10 digits pushed a value near the float maximum past + # it; that text would read back as Inf. Write it exactly instead. + text, rounded = repr(value), value + # Decide the integer form on the ROUNDED value, so a value that rounds to + # an integer is written the same way the second time (idempotence). + if rounded == int(rounded) and abs(rounded) < 1e15: + return str(int(rounded)) + return text def _write_bus_row(b: Bus) -> str: diff --git a/agentigrid/claude_code_changes.md b/agentigrid/claude_code_changes.md index 17433ecc..523b2651 100644 --- a/agentigrid/claude_code_changes.md +++ b/agentigrid/claude_code_changes.md @@ -24,6 +24,14 @@ Also: CRLF input is normalised on read and the writer always emits LF; the `func | `agentigrid/exago_parsers/matpower_writer.py` | Fixes 3, 4 (header); casename/function line; LF output | | `tests/grid_files/test_matpower_hardening.py` | New (29 tests; 22 fail without the fix) | +**Follow-up (property-based testing).** Randomised round-trip tests then found three more defects: + +- A finite value near the float maximum was written with 10 significant digits that round past it, and read back as `Inf`. The writer now falls back to the exact representation. +- A value that rounds to an integer (for example `9999999999.5`) was written as `1e+10` the first time and `10000000000` the second, so writing was not idempotent. The integer form is now decided on the rounded value. +- `mpc.baseMVA` was read with a digits-and-dots pattern. `1e2` (or `1e+15`, which the writer itself emits) was silently read as `1`, and negative values were "not found". It now accepts any numeric literal (sign, exponent, `Inf`/`NaN`) on an `mpc.baseMVA` line, ignores the text in comments, and raises `ValueError` for a non-numeric value. + +18 more tests; 12 of them fail without these fixes. + --- ## MATPOWER cell arrays keep "};" — ExaGO reads genfuel again (2026-10-03) diff --git a/agentigrid/tests/grid_files/test_matpower_hardening.py b/agentigrid/tests/grid_files/test_matpower_hardening.py index 98c0d857..0ffe467f 100644 --- a/agentigrid/tests/grid_files/test_matpower_hardening.py +++ b/agentigrid/tests/grid_files/test_matpower_hardening.py @@ -228,3 +228,52 @@ def test_writer_does_not_mutate_its_input(tmp_path): before = copy.deepcopy(net) write_matpower(net, tmp_path / "out.m") assert net == before + + +# --- number formatting and baseMVA (found by property-based testing) --------- + +@pytest.mark.parametrize( + "value", + [9999999999.5, 123456789012.345, 1e15, 1e16, 0.1 + 0.2, 5e-324, + 1.7976931345e308, -1.7976931348623157e308], + ids=["rounds-to-int", "11-digits", "1e15", "1e16", "0.3", "subnormal", + "near-max", "-max"], +) +def test_number_round_trip_is_close_finite_and_idempotent(tmp_path, value): + net = parse_matpower(_write(tmp_path, _case())) + net.buses[1].Pd = value + a, b = tmp_path / "a.m", tmp_path / "b.m" + write_matpower(net, a) + back = parse_matpower(a) + assert math.isfinite(back.buses[1].Pd) + assert math.isclose(back.buses[1].Pd, value, rel_tol=1e-9, abs_tol=1e-12) + write_matpower(back, b) + assert a.read_text() == b.read_text() + + +@pytest.mark.parametrize( + "text, expected", + [("1e2", 100.0), ("100.0", 100.0), ("-1", -1.0), ("1e+16", 1e16), ("Inf", math.inf)], +) +def test_basemva_accepts_any_numeric_literal(tmp_path, text, expected): + net = parse_matpower(_write(tmp_path, _case().replace("mpc.baseMVA = 100;", f"mpc.baseMVA = {text};"))) + assert net.baseMVA == expected + + +def test_basemva_in_a_comment_is_ignored(tmp_path): + text = _case().replace("%TINY test case\n", "%TINY test case\n% mpc.baseMVA = 5;\n") + assert parse_matpower(_write(tmp_path, text)).baseMVA == 100.0 + + +def test_basemva_non_numeric_raises(tmp_path): + with pytest.raises(ValueError, match="baseMVA"): + parse_matpower(_write(tmp_path, _case().replace("mpc.baseMVA = 100;", "mpc.baseMVA = abc;"))) + + +@pytest.mark.parametrize("value", [1e16, -1.0, 1.7976931345e308]) +def test_basemva_round_trip(tmp_path, value): + net = parse_matpower(_write(tmp_path, _case())) + net.baseMVA = value + out = tmp_path / "out.m" + write_matpower(net, out) + assert math.isclose(parse_matpower(out).baseMVA, value, rel_tol=1e-9)