Fix GFS shortName regression: use ecCodes names, not cfgrib names

Mirror of the WA2 fix — same bug, same change, synced to stay in lockstep
until init_sources is extracted to a shared package.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

src/init_sources/gfs.py

@@ -144,30 +144,48 @@ def _download_grib2(
 # GRIB parsing
 # ---------------------------------------------------------------------------
 
-def _open_grib_surface(path: str, gfs_short_name: str) -> Any:
+def _open_grib_surface(path: str, ecmwf_short_name: str) -> Any:
     """Return an xarray.DataArray for a single surface variable from GRIB2.
 
-    cfgrib requires ``filter_by_keys`` to select which GRIB message layer
-    to decode. For surface variables we filter by the short name plus the
-    ``heightAboveGround`` or ``meanSea`` type of level as appropriate.
+    ``ecmwf_short_name`` is the ecCodes shortName (e.g. ``2t``, ``10u``,
+    ``prmsl``, ``tp``) — what GRIB messages are indexed by, NOT cfgrib's
+    output variable name (e.g. ``t2m``, ``u10``). cfgrib's
+    ``filter_by_keys`` takes ecCodes names; getting this wrong returns an
+    empty dataset silently, which was the Phase-2-take-1 failure.
+
+    The filter is typeOfLevel + shortName (+ level for heightAboveGround).
+    After filtering there's exactly one message remaining, so we return the
+    first data_var regardless of cfgrib's rename.
     """
     import xarray
-    # cfgrib's "typeOfLevel" filters pick the right surface layer. GFS stores
-    # t2m/u10/v10 at ``heightAboveGround`` (2m and 10m), ``prmsl`` at
-    # ``meanSea``, and ``tp`` at ``surface``.
+    # GFS stores ``2t``/``10u``/``10v`` at ``heightAboveGround`` (2m and
+    # 10m), ``prmsl`` at ``meanSea``, and ``tp`` at ``surface``.
     level_map = {
-        "t2m": ("heightAboveGround", 2),
-        "u10": ("heightAboveGround", 10),
-        "v10": ("heightAboveGround", 10),
+        "2t":    ("heightAboveGround", 2),
+        "10u":   ("heightAboveGround", 10),
+        "10v":   ("heightAboveGround", 10),
         "prmsl": ("meanSea", 0),
         "tp":    ("surface", 0),
     }
-    type_of_level, level = level_map.get(gfs_short_name, ("surface", 0))
-    filters = {"typeOfLevel": type_of_level, "shortName": gfs_short_name}
+    type_of_level, level = level_map.get(ecmwf_short_name, ("surface", 0))
+    filters = {"typeOfLevel": type_of_level, "shortName": ecmwf_short_name}
     if type_of_level == "heightAboveGround":
         filters["level"] = level
-    return xarray.open_dataset(path, engine="cfgrib",
-                                backend_kwargs={"filter_by_keys": filters})[gfs_short_name]
+    ds = xarray.open_dataset(
+        path, engine="cfgrib",
+        backend_kwargs={"filter_by_keys": filters},
+    )
+    names = list(ds.data_vars)
+    if not names:
+        raise RuntimeError(
+            f"cfgrib returned 0 variables for shortName={ecmwf_short_name!r}, "
+            f"typeOfLevel={type_of_level!r}. Filter likely wrong — check "
+            f"ecCodes naming (2t vs t2m, 10u vs u10, etc.)."
+        )
+    if len(names) != 1:
+        log.warning("Multiple data vars (%s) after GRIB filter for %s; "
+                    "using first", names, ecmwf_short_name)
+    return ds[names[0]]
 
 
 def _open_grib_pressure_levels(

src/init_sources/variable_mapping.py

@@ -36,10 +36,20 @@ from typing import Dict, Tuple
 # step — cfgrib exposes ``tp`` in kg/m² which is numerically the same as mm of
 # liquid water. ERA5 reports precipitation in m. We divide by 1000 to match.
 
+#
+# ``ecCodes`` shortName differs from cfgrib's output variable name for a
+# handful of surface fields. The GRIB *message* is indexed by the ecCodes
+# shortName (``2t``, ``10u``, ``10v``, ``prmsl``, ``tp``) which is what
+# ``filter_by_keys`` takes. When cfgrib materialises the filtered message as
+# an xarray variable, it uses its own naming (``t2m``, ``u10``, ``v10``,
+# ``prmsl``, ``tp``). We carry the ecCodes shortName here because the filter
+# is load-bearing — get this wrong and filter_by_keys returns an empty
+# dataset, which was the Phase-2-take-1 failure mode.
+
 SURFACE_VARS: Dict[str, Tuple[str, Tuple[float, float]]] = {
-    "2m_temperature":               ("t2m",   (1.0,    0.0)),
-    "10m_u_component_of_wind":      ("u10",   (1.0,    0.0)),
-    "10m_v_component_of_wind":      ("v10",   (1.0,    0.0)),
+    "2m_temperature":               ("2t",    (1.0,    0.0)),
+    "10m_u_component_of_wind":      ("10u",   (1.0,    0.0)),
+    "10m_v_component_of_wind":      ("10v",   (1.0,    0.0)),
     "mean_sea_level_pressure":      ("prmsl", (1.0,    0.0)),
     "total_precipitation_6hr":      ("tp",    (1e-3,   0.0)),  # mm → m
 }

tests/test_init_sources/test_variable_mapping.py

@@ -45,6 +45,25 @@ class TestSurfaceVarsLookup:
         assert scale == 1.0
         assert offset == 0.0
 
+    def test_surface_short_names_are_eccodes_not_cfgrib(self):
+        # Phase 2 regression: cfgrib's ``filter_by_keys`` takes the ecCodes
+        # shortName (``2t``, ``10u``, ``10v``, etc.), NOT cfgrib's output
+        # xarray variable name (``t2m``, ``u10``, ``v10``). Getting this
+        # wrong returns an empty Dataset silently — the first smoke run
+        # burned ~$3 of A100 discovering it.
+        expected_eccodes_names = {
+            "2m_temperature":          "2t",
+            "10m_u_component_of_wind": "10u",
+            "10m_v_component_of_wind": "10v",
+            "mean_sea_level_pressure": "prmsl",
+            "total_precipitation_6hr": "tp",
+        }
+        for era5_name, eccodes in expected_eccodes_names.items():
+            assert vm.SURFACE_VARS[era5_name][0] == eccodes, (
+                f"{era5_name!r}: must use ecCodes shortName "
+                f"{eccodes!r}, not cfgrib output name"
+            )
+
 
 class TestPressureLevelVarsLookup:
     def test_geopotential_converts_gpm_to_m2s2(self):
@@ -87,7 +106,8 @@ class TestPressureLevelsSelection:
 
 class TestHelpers:
     def test_gfs_short_name_surface(self):
-        assert vm.gfs_short_name("2m_temperature") == "t2m"
+        # ecCodes shortName convention — see test_surface_short_names_are_eccodes_not_cfgrib.
+        assert vm.gfs_short_name("2m_temperature") == "2t"
         assert vm.gfs_short_name("mean_sea_level_pressure") == "prmsl"
 
     def test_gfs_short_name_pressure_level(self):