AgentiGrid: MATPOWER reader/writer hardening (8 defects)

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
 
@@ -22,10 +23,30 @@
 
 _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.<section> = [  or  mpc.<section> = {
 _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 +60,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 +127,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 +143,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 +248,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,12 +260,21 @@ 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)
     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.")
@@ -222,36 +300,33 @@ 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
             # Keep the MATLAB terminator "};" -- ExaGO's reader finds the end of
             # 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 +340,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(

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,12 +17,30 @@
 
 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."""
-    if value == int(value) and abs(value) < 1e15:
-        return str(int(value))
-    return f"{value:.10g}"
+    """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"
+    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:
@@ -31,7 +51,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 +113,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 +182,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)

agentigrid/claude_code_changes.md

@@ -4,6 +4,36 @@ 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) |
+
+**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)
 
 **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 `}`.