From 8b78cb008b3b5da5647002769286b5a5d4ca4296 Mon Sep 17 00:00:00 2001 From: Janice Kim Date: Thu, 2 Oct 2025 15:58:39 -0400 Subject: [PATCH 1/8] NDSL Issue#64: - Adding helper functions for loading f90nml.Namelist from file and converting to dict. - Modifying conftest.py and ParallelTranslate layout() to rely more on f90nml.Namelist. - Adding temporary 'f90nml_namelist_only' flag to conftest.py to toggle testing using 1. only f90nml.Namelist, or 2. f90nml.Namelist and ndsl.Namelist (Changes will eventually facilitate removal of current ndsl.Namelist class) --- ndsl/stencils/testing/conftest.py | 64 ++++++++----- ndsl/stencils/testing/parallel_translate.py | 13 ++- ndsl/utils.py | 99 +++++++++++++++++++++ 3 files changed, 154 insertions(+), 22 deletions(-) diff --git a/ndsl/stencils/testing/conftest.py b/ndsl/stencils/testing/conftest.py index 4b62dc5f..5bc4a0b7 100644 --- a/ndsl/stencils/testing/conftest.py +++ b/ndsl/stencils/testing/conftest.py @@ -2,10 +2,10 @@ import re from typing import Optional, Tuple -import f90nml import pytest import xarray as xr import yaml +from f90nml import Namelist from ndsl import CompilationConfig, StencilConfig, StencilFactory from ndsl.comm.communicator import ( @@ -16,11 +16,14 @@ from ndsl.comm.mpi import MPI, MPIComm from ndsl.comm.partitioner import CubedSpherePartitioner, TilePartitioner from ndsl.dsl.dace.dace_config import DaceConfig -from ndsl.namelist import Namelist + +# TODO: Remove NdslNamelist import after Issue#64 is resolved. +from ndsl.namelist import Namelist as NdslNamelist from ndsl.stencils.testing.grid import Grid # type: ignore from ndsl.stencils.testing.parallel_translate import ParallelTranslate from ndsl.stencils.testing.savepoint import SavepointCase, dataset_to_dict from ndsl.stencils.testing.translate import TranslateGrid +from ndsl.utils import grid_params_from_f90nml, load_f90nml def pytest_addoption(parser): @@ -73,6 +76,12 @@ def pytest_addoption(parser): default=1, help="How many indices of failures to print from worst to best. Default to 1.", ) + parser.addoption( + "--f90nml_namelist_only", + action="store_true", + default=False, + help="When specified, translate tests will use f90nml.Namelist exclusively, without ndsl.Namelist. (Default to False -- which means ndsl.Namelist is used). This flag should be removed after NDSL Issue#64 has been resolved.", + ) parser.addoption( "--grid", action="store", @@ -224,10 +233,6 @@ def get_savepoint_restriction(metafunc): return int(svpt) if svpt else None -def get_namelist(namelist_filename): - return Namelist.from_f90nml(f90nml.read(namelist_filename)) - - def get_config(backend: str, communicator: Optional[Communicator]): stencil_config = StencilConfig( compilation_config=CompilationConfig( @@ -243,14 +248,16 @@ def get_config(backend: str, communicator: Optional[Communicator]): def sequential_savepoint_cases(metafunc, data_path, namelist_filename, *, backend: str): savepoint_names = get_sequential_savepoint_names(metafunc, data_path) - namelist = get_namelist(namelist_filename) + namelist = load_f90nml(namelist_filename) + grid_params = grid_params_from_f90nml(namelist) stencil_config = get_config(backend, None) - ranks = get_ranks(metafunc, namelist.layout) + ranks = get_ranks(metafunc, grid_params["layout"]) savepoint_to_replay = get_savepoint_restriction(metafunc) grid_mode = metafunc.config.getoption("grid") topology_mode = metafunc.config.getoption("topology") sort_report = metafunc.config.getoption("sort_report") no_report = metafunc.config.getoption("no_report") + f90nml_namelist_only = metafunc.config.getoption("f90nml_namelist_only") return _savepoint_cases( savepoint_names, ranks, @@ -263,6 +270,7 @@ def sequential_savepoint_cases(metafunc, data_path, namelist_filename, *, backen topology_mode, sort_report=sort_report, no_report=no_report, + f90nml_namelist_only=f90nml_namelist_only, ) @@ -278,15 +286,17 @@ def _savepoint_cases( topology_mode: bool, sort_report: str, no_report: bool, + f90nml_namelist_only: bool, ): + grid_params = grid_params_from_f90nml(namelist) return_list = [] for rank in ranks: if grid_mode == "default": grid = Grid._make( - namelist.npx, - namelist.npy, - namelist.npz, - namelist.layout, + grid_params["npx"], + grid_params["npy"], + grid_params["npz"], + grid_params["layout"], rank, backend, ) @@ -297,12 +307,12 @@ def _savepoint_cases( grid = TranslateGrid( dataset_to_dict(ds_grid.isel(rank=rank)), rank=rank, - layout=namelist.layout, + layout=grid_params["layout"], backend=backend, ).python_grid() if grid_mode == "compute": compute_grid_data( - grid, namelist, backend, namelist.layout, topology_mode + grid, grid_params, backend, grid_params["layout"], topology_mode ) else: raise NotImplementedError(f"Grid mode {grid_mode} is unknown.") @@ -312,6 +322,12 @@ def _savepoint_cases( grid_indexing=grid.grid_indexing, ) for test_name in sorted(list(savepoint_names)): + # TODO REMOVE this conversion from f90nml.Namelist to ndsl.Namelist + # after ndsl.Namelist is removed + if not f90nml_namelist_only: + if type(namelist) is not NdslNamelist: + namelist = NdslNamelist.from_f90nml(namelist) + testobj = get_test_class_instance( test_name, grid, namelist, stencil_factory ) @@ -337,11 +353,11 @@ def _savepoint_cases( return return_list -def compute_grid_data(grid, namelist, backend, layout, topology_mode): +def compute_grid_data(grid, grid_params, backend, layout, topology_mode): grid.make_grid_data( - npx=namelist.npx, - npy=namelist.npy, - npz=namelist.npz, + npx=grid_params["npx"], + npy=grid_params["npy"], + npz=grid_params["npz"], communicator=get_communicator(MPIComm(), layout, topology_mode), backend=backend, ) @@ -350,15 +366,17 @@ def compute_grid_data(grid, namelist, backend, layout, topology_mode): def parallel_savepoint_cases( metafunc, data_path, namelist_filename, mpi_rank, *, backend: str, comm ): - namelist = get_namelist(namelist_filename) + namelist = load_f90nml(namelist_filename) + grid_params = grid_params_from_f90nml(namelist) topology_mode = metafunc.config.getoption("topology") sort_report = metafunc.config.getoption("sort_report") no_report = metafunc.config.getoption("no_report") - communicator = get_communicator(comm, namelist.layout, topology_mode) + communicator = get_communicator(comm, grid_params["layout"], topology_mode) stencil_config = get_config(backend, communicator) savepoint_names = get_parallel_savepoint_names(metafunc, data_path) grid_mode = metafunc.config.getoption("grid") savepoint_to_replay = get_savepoint_restriction(metafunc) + f90nml_namelist_only = metafunc.config.getoption("f90nml_namelist_only") return _savepoint_cases( savepoint_names, [mpi_rank], @@ -371,6 +389,7 @@ def parallel_savepoint_cases( topology_mode, sort_report=sort_report, no_report=no_report, + f90nml_namelist_only=f90nml_namelist_only, ) @@ -388,7 +407,10 @@ def generate_sequential_stencil_tests(metafunc, *, backend: str): metafunc.config ) savepoint_cases = sequential_savepoint_cases( - metafunc, data_path, namelist_filename, backend=backend + metafunc, + data_path, + namelist_filename, + backend=backend, ) metafunc.parametrize( "case", savepoint_cases, ids=[str(item) for item in savepoint_cases] diff --git a/ndsl/stencils/testing/parallel_translate.py b/ndsl/stencils/testing/parallel_translate.py index 7df16a17..827bf22e 100644 --- a/ndsl/stencils/testing/parallel_translate.py +++ b/ndsl/stencils/testing/parallel_translate.py @@ -7,11 +7,15 @@ from ndsl.constants import HORIZONTAL_DIMS, N_HALO_DEFAULT, X_DIMS, Y_DIMS from ndsl.dsl import gt4py_utils as utils + +# TODO: Remove once ndsl.Namelist is gone (Issue#64) +from ndsl.namelist import Namelist as NdslNamelist from ndsl.quantity import Quantity from ndsl.stencils.testing.translate import ( TranslateFortranData2Py, read_serialized_data, ) +from ndsl.utils import grid_params_from_f90nml class ParallelTranslate: @@ -129,7 +133,14 @@ def rank_grids(self): @property def layout(self): - return self.namelist.layout + # TODO: Once ndsl.namelist.Namelist is gone (Issue#64), + # remove this check in favor of f90nml.namelist.Namelist + if type(self.namelist is NdslNamelist): + return self.namelist.layout + + # Assumption: namelist is f90nml.namelist.Namelist + grid_params = grid_params_from_f90nml(self.namelist) + return grid_params["layout"] def compute_sequential(self, inputs_list, communicator_list): """Compute the outputs while iterating over a set of communicator diff --git a/ndsl/utils.py b/ndsl/utils.py index 36316680..d070f24c 100644 --- a/ndsl/utils.py +++ b/ndsl/utils.py @@ -1,6 +1,7 @@ from enum import EnumMeta from typing import Iterable, Sequence, Tuple, TypeVar, Union +import f90nml import numpy as np import ndsl.constants as constants @@ -111,3 +112,101 @@ def safe_mpi_allocate( if __debug__ and cp and isinstance(array, cp.ndarray): raise RuntimeError("cupy allocation might not be MPI-safe") return array + + +######################################################## +# Helpers for loading and working with Fortran Namelists +# TODO: Consider moving these to a separate utils/namelist.py + +DEFAULT_GRID_NML_GROUPS = ["fv_core_nml"] + + +def flatten_nml_to_dict(nml: f90nml.Namelist) -> dict: + """Returns a flattened dict version of a f90nml.namelist.Namelist + + Args: + nml: f90nml.Namelist + """ + nml_dict = dict(nml) + for name, value in nml_dict.items(): + if isinstance(value, f90nml.Namelist): + nml_dict[name] = flatten_nml_to_dict(value) + flatter_namelist = {} + for key, value in nml_dict.items(): + if isinstance(value, dict): + for subkey, subvalue in value.items(): + if subkey in flatter_namelist: + raise ValueError( + "Cannot flatten this namelist, duplicate keys: " + subkey + ) + flatter_namelist[subkey] = subvalue + else: + flatter_namelist[key] = value + return flatter_namelist + + +def load_f90nml(namelist_path: str) -> f90nml.Namelist: + """Loads a Fortran namelist given its path and return a f90nml.Namelist + + Args: + namelist_path: Path to the Fortran namelist file + """ + return f90nml.read(namelist_path) + + +def load_f90nml_as_dict( + namelist_path: str, + flatten: bool = True, + target_groups=None, +) -> dict: + """Loads a Fortran namelist given its path and returns a + dict representation. If target_groups are specified, then + the dict is created using only those groups. + + Args: + namelist_path: Path to the Fortran namelist file + flatten: If True, flattens the loaded namelist (without groups) before + returning it. (Default: True) Otherwise, it returns the f90nml.Namelist + dict representation. + target_groups: If 'None' is specified, then all groups are + considered. (Default: None) Otherwise, only parameters + from the specified groups are considered. + """ + nml = load_f90nml(namelist_path) + return f90nml_as_dict(nml, flatten=flatten, target_groups=target_groups) + + +def f90nml_as_dict( + nml: f90nml.Namelist, + flatten: bool = True, + target_groups=None, +) -> dict: + """Uses a f90nml.Namelist and returns a dict representation. + If target_groups are specified, then the dict is created using only those + groups. The return dicts can be flattened further to remove the group + information or keep the group information. + + Args: + namelist_path: Path to the Fortran namelist file + flatten: If True, flattens the loaded namelist (without groups) before + returning it. (Default: True) Otherwise, it returns the f90nml.Namelist + dict representation. + target_groups: If 'None' is specified, then all groups are + considered. (Default: None) Otherwise, only parameters + from the specified groups are considered. + """ + if target_groups is not None: + extracted_groups = f90nml.Namelist() + for group in target_groups: + if group in nml: + extracted_groups[group] = nml[group] + else: + extracted_groups = nml + + if flatten: + return flatten_nml_to_dict(extracted_groups) + return extracted_groups.todict() + + +def grid_params_from_f90nml(nml): + return f90nml_as_dict(nml, flatten=True, target_groups=DEFAULT_GRID_NML_GROUPS) From 3311101e81947ef06aa50123ca5d19755dc74c03 Mon Sep 17 00:00:00 2001 From: Janice Kim Date: Fri, 3 Oct 2025 11:05:18 -0400 Subject: [PATCH 2/8] Tagging with Issue#64 where appropriate --- ndsl/stencils/testing/conftest.py | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/ndsl/stencils/testing/conftest.py b/ndsl/stencils/testing/conftest.py index 5bc4a0b7..ad238c73 100644 --- a/ndsl/stencils/testing/conftest.py +++ b/ndsl/stencils/testing/conftest.py @@ -257,7 +257,10 @@ def sequential_savepoint_cases(metafunc, data_path, namelist_filename, *, backen topology_mode = metafunc.config.getoption("topology") sort_report = metafunc.config.getoption("sort_report") no_report = metafunc.config.getoption("no_report") + + # Temporary flag (Issue#64): TODO Remove once ndsl.Namelist is gone. f90nml_namelist_only = metafunc.config.getoption("f90nml_namelist_only") + return _savepoint_cases( savepoint_names, ranks, @@ -270,7 +273,7 @@ def sequential_savepoint_cases(metafunc, data_path, namelist_filename, *, backen topology_mode, sort_report=sort_report, no_report=no_report, - f90nml_namelist_only=f90nml_namelist_only, + f90nml_namelist_only=f90nml_namelist_only, # Issue#64: tmp flag ) @@ -286,7 +289,7 @@ def _savepoint_cases( topology_mode: bool, sort_report: str, no_report: bool, - f90nml_namelist_only: bool, + f90nml_namelist_only: bool, # Issue#64: tmp flag ): grid_params = grid_params_from_f90nml(namelist) return_list = [] @@ -322,8 +325,8 @@ def _savepoint_cases( grid_indexing=grid.grid_indexing, ) for test_name in sorted(list(savepoint_names)): - # TODO REMOVE this conversion from f90nml.Namelist to ndsl.Namelist - # after ndsl.Namelist is removed + # Temporary check (Issue#64): TODO Remove check and conversion from + # f90nml.Namelist to ndsl.Namelist after ndsl.Namelist is removed if not f90nml_namelist_only: if type(namelist) is not NdslNamelist: namelist = NdslNamelist.from_f90nml(namelist) @@ -376,7 +379,10 @@ def parallel_savepoint_cases( savepoint_names = get_parallel_savepoint_names(metafunc, data_path) grid_mode = metafunc.config.getoption("grid") savepoint_to_replay = get_savepoint_restriction(metafunc) + + # Temporary flag (Issue#64): TODO Remove once ndsl.Namelist is gone. f90nml_namelist_only = metafunc.config.getoption("f90nml_namelist_only") + return _savepoint_cases( savepoint_names, [mpi_rank], @@ -389,7 +395,7 @@ def parallel_savepoint_cases( topology_mode, sort_report=sort_report, no_report=no_report, - f90nml_namelist_only=f90nml_namelist_only, + f90nml_namelist_only=f90nml_namelist_only, # Issue#64: tmp flag ) From 17cfd67a1f5ba65fd852a66c566836382becdaee Mon Sep 17 00:00:00 2001 From: Janice Kim Date: Fri, 3 Oct 2025 12:39:04 -0400 Subject: [PATCH 3/8] Changing path str to libpath.Path --- ndsl/stencils/testing/conftest.py | 21 +++++++++++---------- ndsl/stencils/testing/savepoint.py | 20 +++++++++----------- ndsl/utils.py | 5 +++-- 3 files changed, 23 insertions(+), 23 deletions(-) diff --git a/ndsl/stencils/testing/conftest.py b/ndsl/stencils/testing/conftest.py index ad238c73..3fdf7720 100644 --- a/ndsl/stencils/testing/conftest.py +++ b/ndsl/stencils/testing/conftest.py @@ -1,5 +1,6 @@ import os import re +from pathlib import Path from typing import Optional, Tuple import pytest @@ -133,9 +134,9 @@ def data_path(pytestconfig): return data_path_and_namelist_filename_from_config(pytestconfig) -def data_path_and_namelist_filename_from_config(config) -> Tuple[str, str]: - data_path = config.getoption("data_path") - namelist_filename = os.path.join(data_path, "input.nml") +def data_path_and_namelist_filename_from_config(config) -> Tuple[Path, Path]: + data_path = Path(config.getoption("data_path")) + namelist_filename = data_path / "input.nml" return data_path, namelist_filename @@ -284,7 +285,7 @@ def _savepoint_cases( stencil_config, namelist: Namelist, backend: str, - data_path: str, + data_path: Path, grid_mode: str, topology_mode: bool, sort_report: str, @@ -304,9 +305,9 @@ def _savepoint_cases( backend, ) elif grid_mode == "file" or grid_mode == "compute": - ds_grid: xr.Dataset = xr.open_dataset( - os.path.join(data_path, "Grid-Info.nc") - ).isel(savepoint=0) + ds_grid: xr.Dataset = xr.open_dataset(data_path / "Grid-Info.nc").isel( + savepoint=0 + ) grid = TranslateGrid( dataset_to_dict(ds_grid.isel(rank=rank)), rank=rank, @@ -334,9 +335,9 @@ def _savepoint_cases( testobj = get_test_class_instance( test_name, grid, namelist, stencil_factory ) - n_calls = xr.open_dataset( - os.path.join(data_path, f"{test_name}-In.nc") - ).sizes["savepoint"] + n_calls = xr.open_dataset(data_path / f"{test_name}-In.nc").sizes[ + "savepoint" + ] if savepoint_to_replay is not None: savepoint_iterator = range(savepoint_to_replay, savepoint_to_replay + 1) else: diff --git a/ndsl/stencils/testing/savepoint.py b/ndsl/stencils/testing/savepoint.py index 68a52fad..af43b5a3 100644 --- a/ndsl/stencils/testing/savepoint.py +++ b/ndsl/stencils/testing/savepoint.py @@ -1,5 +1,5 @@ import dataclasses -import os +from pathlib import Path from typing import Dict, Protocol, Union import numpy as np @@ -22,7 +22,7 @@ def _process_if_scalar(value: np.ndarray) -> Union[np.ndarray, float, int]: class DataLoader: - def __init__(self, rank: int, data_path: str): + def __init__(self, rank: int, data_path: Path): self._data_path = data_path self._rank = rank @@ -33,7 +33,7 @@ def load( i_call: int = 0, ) -> Dict[str, Union[np.ndarray, float, int]]: return dataset_to_dict( - xr.open_dataset(os.path.join(self._data_path, f"{name}{postfix}.nc")) + xr.open_dataset(self._data_path / f"{name}{postfix}.nc") .isel(rank=self._rank) .isel(savepoint=i_call) ) @@ -54,7 +54,7 @@ class SavepointCase: """ savepoint_name: str - data_dir: str + data_dir: Path i_call: int testobj: Translate grid: Grid @@ -67,16 +67,16 @@ def __str__(self): @property def exists(self) -> bool: return ( - xr.open_dataset( - os.path.join(self.data_dir, f"{self.savepoint_name}-In.nc") - ).sizes["rank"] + xr.open_dataset(self.data_dir / f"{self.savepoint_name}-In.nc").sizes[ + "rank" + ] > self.grid.rank ) @property def ds_in(self) -> xr.Dataset: return ( - xr.open_dataset(os.path.join(self.data_dir, f"{self.savepoint_name}-In.nc")) + xr.open_dataset(self.data_dir / f"{self.savepoint_name}-In.nc") .isel(rank=self.grid.rank) .isel(savepoint=self.i_call) ) @@ -84,9 +84,7 @@ def ds_in(self) -> xr.Dataset: @property def ds_out(self) -> xr.Dataset: return ( - xr.open_dataset( - os.path.join(self.data_dir, f"{self.savepoint_name}-Out.nc") - ) + xr.open_dataset(self.data_dir / f"{self.savepoint_name}-Out.nc") .isel(rank=self.grid.rank) .isel(savepoint=self.i_call) ) diff --git a/ndsl/utils.py b/ndsl/utils.py index d070f24c..6df56144 100644 --- a/ndsl/utils.py +++ b/ndsl/utils.py @@ -1,4 +1,5 @@ from enum import EnumMeta +from pathlib import Path from typing import Iterable, Sequence, Tuple, TypeVar, Union import f90nml @@ -145,7 +146,7 @@ def flatten_nml_to_dict(nml: f90nml.Namelist) -> dict: return flatter_namelist -def load_f90nml(namelist_path: str) -> f90nml.Namelist: +def load_f90nml(namelist_path: Path) -> f90nml.Namelist: """Loads a Fortran namelist given its path and return a f90nml.Namelist Args: @@ -155,7 +156,7 @@ def load_f90nml(namelist_path: str) -> f90nml.Namelist: def load_f90nml_as_dict( - namelist_path: str, + namelist_path: Path, flatten: bool = True, target_groups=None, ) -> dict: From a7219997b457fe10b790616918fbd69382738859 Mon Sep 17 00:00:00 2001 From: Janice Kim Date: Mon, 6 Oct 2025 13:34:01 -0400 Subject: [PATCH 4/8] Fixes from PR#246 feedback --- ndsl/stencils/testing/conftest.py | 34 ++++++++++++++------- ndsl/stencils/testing/parallel_translate.py | 2 +- ndsl/utils.py | 23 +++++++++++--- 3 files changed, 42 insertions(+), 17 deletions(-) diff --git a/ndsl/stencils/testing/conftest.py b/ndsl/stencils/testing/conftest.py index 3fdf7720..bdeec243 100644 --- a/ndsl/stencils/testing/conftest.py +++ b/ndsl/stencils/testing/conftest.py @@ -78,10 +78,10 @@ def pytest_addoption(parser): help="How many indices of failures to print from worst to best. Default to 1.", ) parser.addoption( - "--f90nml_namelist_only", - action="store_true", - default=False, - help="When specified, translate tests will use f90nml.Namelist exclusively, without ndsl.Namelist. (Default to False -- which means ndsl.Namelist is used). This flag should be removed after NDSL Issue#64 has been resolved.", + "--legacy_namelist_support", + action="store", + default="True", + help="Keeps support for `ndsl.Namelist` in translate tests (which we are trying to get rid off, see NDSL issue #64. Defaults to True.", ) parser.addoption( "--grid", @@ -234,6 +234,18 @@ def get_savepoint_restriction(metafunc): return int(svpt) if svpt else None +def get_legacy_namelist_support(metafunc): + use_legacy = metafunc.config.getoption("legacy_namelist_support").lower() + if use_legacy in ("true", "t", "1", "yes", "y"): + return True + elif use_legacy in ("false", "f", "0", "no", "n"): + return False + else: + raise ValueError( + f"Invalid value for legacy_namelist_support flag: {use_legacy}" + ) + + def get_config(backend: str, communicator: Optional[Communicator]): stencil_config = StencilConfig( compilation_config=CompilationConfig( @@ -260,7 +272,7 @@ def sequential_savepoint_cases(metafunc, data_path, namelist_filename, *, backen no_report = metafunc.config.getoption("no_report") # Temporary flag (Issue#64): TODO Remove once ndsl.Namelist is gone. - f90nml_namelist_only = metafunc.config.getoption("f90nml_namelist_only") + legacy_namelist_support = get_legacy_namelist_support(metafunc) return _savepoint_cases( savepoint_names, @@ -274,7 +286,7 @@ def sequential_savepoint_cases(metafunc, data_path, namelist_filename, *, backen topology_mode, sort_report=sort_report, no_report=no_report, - f90nml_namelist_only=f90nml_namelist_only, # Issue#64: tmp flag + legacy_namelist_support=legacy_namelist_support, # Issue#64: tmp flag ) @@ -290,7 +302,7 @@ def _savepoint_cases( topology_mode: bool, sort_report: str, no_report: bool, - f90nml_namelist_only: bool, # Issue#64: tmp flag + legacy_namelist_support: bool, # Issue#64: tmp flag ): grid_params = grid_params_from_f90nml(namelist) return_list = [] @@ -328,8 +340,8 @@ def _savepoint_cases( for test_name in sorted(list(savepoint_names)): # Temporary check (Issue#64): TODO Remove check and conversion from # f90nml.Namelist to ndsl.Namelist after ndsl.Namelist is removed - if not f90nml_namelist_only: - if type(namelist) is not NdslNamelist: + if legacy_namelist_support: + if not isinstance(namelist, NdslNamelist): namelist = NdslNamelist.from_f90nml(namelist) testobj = get_test_class_instance( @@ -382,7 +394,7 @@ def parallel_savepoint_cases( savepoint_to_replay = get_savepoint_restriction(metafunc) # Temporary flag (Issue#64): TODO Remove once ndsl.Namelist is gone. - f90nml_namelist_only = metafunc.config.getoption("f90nml_namelist_only") + legacy_namelist_support = get_legacy_namelist_support(metafunc) return _savepoint_cases( savepoint_names, @@ -396,7 +408,7 @@ def parallel_savepoint_cases( topology_mode, sort_report=sort_report, no_report=no_report, - f90nml_namelist_only=f90nml_namelist_only, # Issue#64: tmp flag + legacy_namelist_support=legacy_namelist_support, # Issue#64: tmp flag ) diff --git a/ndsl/stencils/testing/parallel_translate.py b/ndsl/stencils/testing/parallel_translate.py index 827bf22e..2d57d378 100644 --- a/ndsl/stencils/testing/parallel_translate.py +++ b/ndsl/stencils/testing/parallel_translate.py @@ -135,7 +135,7 @@ def rank_grids(self): def layout(self): # TODO: Once ndsl.namelist.Namelist is gone (Issue#64), # remove this check in favor of f90nml.namelist.Namelist - if type(self.namelist is NdslNamelist): + if isinstance(self.namelist, NdslNamelist): return self.namelist.layout # Assumption: namelist is f90nml.namelist.Namelist diff --git a/ndsl/utils.py b/ndsl/utils.py index 6df56144..ae357a7e 100644 --- a/ndsl/utils.py +++ b/ndsl/utils.py @@ -119,6 +119,7 @@ def safe_mpi_allocate( # Helpers for loading and working with Fortran Namelists # TODO: Consider moving these to a separate utils/namelist.py + DEFAULT_GRID_NML_GROUPS = ["fv_core_nml"] @@ -146,6 +147,8 @@ def flatten_nml_to_dict(nml: f90nml.Namelist) -> dict: return flatter_namelist +# TODO: Consider a more universal loader, e.g., load_config(path), +# rather than a f90nml-specific loader (See PR#246). def load_f90nml(namelist_path: Path) -> f90nml.Namelist: """Loads a Fortran namelist given its path and return a f90nml.Namelist @@ -158,7 +161,7 @@ def load_f90nml(namelist_path: Path) -> f90nml.Namelist: def load_f90nml_as_dict( namelist_path: Path, flatten: bool = True, - target_groups=None, + target_groups=list[str] | None, ) -> dict: """Loads a Fortran namelist given its path and returns a dict representation. If target_groups are specified, then @@ -180,7 +183,7 @@ def load_f90nml_as_dict( def f90nml_as_dict( nml: f90nml.Namelist, flatten: bool = True, - target_groups=None, + target_groups=list[str] | None, ) -> dict: """Uses a f90nml.Namelist and returns a dict representation. If target_groups are specified, then the dict is created using only those @@ -188,7 +191,7 @@ def f90nml_as_dict( information or keep the group information. Args: - namelist_path: Path to the Fortran namelist file + nml: f90nml.Namelist flatten: If True, flattens the loaded namelist (without groups) before returning it. (Default: True) Otherwise, it returns the f90nml.Namelist dict representation. @@ -199,7 +202,7 @@ def f90nml_as_dict( if target_groups is not None: extracted_groups = f90nml.Namelist() for group in target_groups: - if group in nml: + if group in nml.keys(): extracted_groups[group] = nml[group] else: extracted_groups = nml @@ -209,5 +212,15 @@ def f90nml_as_dict( return extracted_groups.todict() -def grid_params_from_f90nml(nml): +def grid_params_from_f90nml(nml: f90nml.Namelist) -> dict: + """Uses a f90nml.Namelist and returns a dict representation + of parameters useful for grid generation. The return dict + will be flattened with key-value pairs from the nml's + DEFAULT_GRID_NML_GROUPS. + + Args: + nml: f90nml.Namelist + """ + # TODO: Consider returning a {Cartesian,CubeSphere}GridParameters class + # rather than a dict (See PR#246). return f90nml_as_dict(nml, flatten=True, target_groups=DEFAULT_GRID_NML_GROUPS) From 03f87d83273255c6dabf5ba64d3a0cfca5a8a4d4 Mon Sep 17 00:00:00 2001 From: Janice Kim Date: Mon, 6 Oct 2025 14:48:52 -0400 Subject: [PATCH 5/8] Changing --legacy_namelist_support to --no_legacy_namelist (PR #246) --- ndsl/stencils/testing/conftest.py | 32 ++++++++++--------------------- 1 file changed, 10 insertions(+), 22 deletions(-) diff --git a/ndsl/stencils/testing/conftest.py b/ndsl/stencils/testing/conftest.py index bdeec243..3ebcec73 100644 --- a/ndsl/stencils/testing/conftest.py +++ b/ndsl/stencils/testing/conftest.py @@ -78,10 +78,10 @@ def pytest_addoption(parser): help="How many indices of failures to print from worst to best. Default to 1.", ) parser.addoption( - "--legacy_namelist_support", - action="store", - default="True", - help="Keeps support for `ndsl.Namelist` in translate tests (which we are trying to get rid off, see NDSL issue #64. Defaults to True.", + "--no_legacy_namelist", + action="store_true", + default=False, + help="Removes support for `ndsl.Namelist` in translate tests (which we are trying to get rid off, see NDSL issue #64). Defaults to False.", ) parser.addoption( "--grid", @@ -234,18 +234,6 @@ def get_savepoint_restriction(metafunc): return int(svpt) if svpt else None -def get_legacy_namelist_support(metafunc): - use_legacy = metafunc.config.getoption("legacy_namelist_support").lower() - if use_legacy in ("true", "t", "1", "yes", "y"): - return True - elif use_legacy in ("false", "f", "0", "no", "n"): - return False - else: - raise ValueError( - f"Invalid value for legacy_namelist_support flag: {use_legacy}" - ) - - def get_config(backend: str, communicator: Optional[Communicator]): stencil_config = StencilConfig( compilation_config=CompilationConfig( @@ -272,7 +260,7 @@ def sequential_savepoint_cases(metafunc, data_path, namelist_filename, *, backen no_report = metafunc.config.getoption("no_report") # Temporary flag (Issue#64): TODO Remove once ndsl.Namelist is gone. - legacy_namelist_support = get_legacy_namelist_support(metafunc) + no_legacy_namelist = metafunc.config.getoption("no_legacy_namelist") return _savepoint_cases( savepoint_names, @@ -286,7 +274,7 @@ def sequential_savepoint_cases(metafunc, data_path, namelist_filename, *, backen topology_mode, sort_report=sort_report, no_report=no_report, - legacy_namelist_support=legacy_namelist_support, # Issue#64: tmp flag + no_legacy_namelist=no_legacy_namelist, # Issue#64: tmp flag ) @@ -302,7 +290,7 @@ def _savepoint_cases( topology_mode: bool, sort_report: str, no_report: bool, - legacy_namelist_support: bool, # Issue#64: tmp flag + no_legacy_namelist: bool, # Issue#64: tmp flag ): grid_params = grid_params_from_f90nml(namelist) return_list = [] @@ -340,7 +328,7 @@ def _savepoint_cases( for test_name in sorted(list(savepoint_names)): # Temporary check (Issue#64): TODO Remove check and conversion from # f90nml.Namelist to ndsl.Namelist after ndsl.Namelist is removed - if legacy_namelist_support: + if not no_legacy_namelist: # This means we use NdslNamelist. if not isinstance(namelist, NdslNamelist): namelist = NdslNamelist.from_f90nml(namelist) @@ -394,7 +382,7 @@ def parallel_savepoint_cases( savepoint_to_replay = get_savepoint_restriction(metafunc) # Temporary flag (Issue#64): TODO Remove once ndsl.Namelist is gone. - legacy_namelist_support = get_legacy_namelist_support(metafunc) + no_legacy_namelist = metafunc.config.getoption("no_legacy_namelist") return _savepoint_cases( savepoint_names, @@ -408,7 +396,7 @@ def parallel_savepoint_cases( topology_mode, sort_report=sort_report, no_report=no_report, - legacy_namelist_support=legacy_namelist_support, # Issue#64: tmp flag + no_legacy_namelist=no_legacy_namelist, # Issue#64: tmp flag ) From a4f273f8978539cc910937d1a6f4839032b4e872 Mon Sep 17 00:00:00 2001 From: Janice Kim Date: Mon, 6 Oct 2025 14:52:54 -0400 Subject: [PATCH 6/8] linting --- ndsl/stencils/testing/conftest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ndsl/stencils/testing/conftest.py b/ndsl/stencils/testing/conftest.py index 3ebcec73..18db30cb 100644 --- a/ndsl/stencils/testing/conftest.py +++ b/ndsl/stencils/testing/conftest.py @@ -328,7 +328,7 @@ def _savepoint_cases( for test_name in sorted(list(savepoint_names)): # Temporary check (Issue#64): TODO Remove check and conversion from # f90nml.Namelist to ndsl.Namelist after ndsl.Namelist is removed - if not no_legacy_namelist: # This means we use NdslNamelist. + if not no_legacy_namelist: # This means we use NdslNamelist. if not isinstance(namelist, NdslNamelist): namelist = NdslNamelist.from_f90nml(namelist) From 6f796f67778be311f1d44399c86cda860e5de84d Mon Sep 17 00:00:00 2001 From: Janice Kim Date: Mon, 6 Oct 2025 16:49:59 -0400 Subject: [PATCH 7/8] type hint fix --- ndsl/utils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ndsl/utils.py b/ndsl/utils.py index ae357a7e..51c1dcf9 100644 --- a/ndsl/utils.py +++ b/ndsl/utils.py @@ -161,7 +161,7 @@ def load_f90nml(namelist_path: Path) -> f90nml.Namelist: def load_f90nml_as_dict( namelist_path: Path, flatten: bool = True, - target_groups=list[str] | None, + target_groups:list[str] | None = None, ) -> dict: """Loads a Fortran namelist given its path and returns a dict representation. If target_groups are specified, then @@ -183,7 +183,7 @@ def load_f90nml_as_dict( def f90nml_as_dict( nml: f90nml.Namelist, flatten: bool = True, - target_groups=list[str] | None, + target_groups:list[str] | None = None, ) -> dict: """Uses a f90nml.Namelist and returns a dict representation. If target_groups are specified, then the dict is created using only those From b2d969ff2545e3e38b3a26d2d109c6c523e4b4dc Mon Sep 17 00:00:00 2001 From: Janice Kim Date: Mon, 6 Oct 2025 16:52:01 -0400 Subject: [PATCH 8/8] linting --- ndsl/utils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ndsl/utils.py b/ndsl/utils.py index 51c1dcf9..60583956 100644 --- a/ndsl/utils.py +++ b/ndsl/utils.py @@ -161,7 +161,7 @@ def load_f90nml(namelist_path: Path) -> f90nml.Namelist: def load_f90nml_as_dict( namelist_path: Path, flatten: bool = True, - target_groups:list[str] | None = None, + target_groups: list[str] | None = None, ) -> dict: """Loads a Fortran namelist given its path and returns a dict representation. If target_groups are specified, then @@ -183,7 +183,7 @@ def load_f90nml_as_dict( def f90nml_as_dict( nml: f90nml.Namelist, flatten: bool = True, - target_groups:list[str] | None = None, + target_groups: list[str] | None = None, ) -> dict: """Uses a f90nml.Namelist and returns a dict representation. If target_groups are specified, then the dict is created using only those