-
Notifications
You must be signed in to change notification settings - Fork 0
merge to latest main #6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 1 commit
3e9d947
60c95bb
4d9bf14
4f86694
e779728
14ea4ee
5a54a0c
eda4f64
e771668
1c4f577
95e3710
62c6760
f5a0a2f
8556b19
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -6,6 +6,7 @@ | |||||||||||||
| from pathlib import Path | ||||||||||||||
| from typing import Optional, Sequence, Union, Dict, List | ||||||||||||||
| import re | ||||||||||||||
| from venv import logger | ||||||||||||||
| import warnings | ||||||||||||||
| import glob | ||||||||||||||
| import sys | ||||||||||||||
|
|
@@ -201,6 +202,7 @@ def realize_regrid_prepare( | |||||||||||||
| *, | ||||||||||||||
| tables_path: Optional[Union[str, Path]] = None, | ||||||||||||||
| time_chunk: Optional[int] = 12, | ||||||||||||||
| mom6_grid: Optional[Dict[str, xr.DataArray]] = None, | ||||||||||||||
| regrid_kwargs: Optional[dict] = None, | ||||||||||||||
| open_kwargs: Optional[dict] = None, | ||||||||||||||
| ) -> xr.Dataset: | ||||||||||||||
|
|
@@ -211,7 +213,7 @@ def realize_regrid_prepare( | |||||||||||||
| """ | ||||||||||||||
| regrid_kwargs = dict(regrid_kwargs or {}) | ||||||||||||||
| open_kwargs = dict(open_kwargs or {}) | ||||||||||||||
|
|
||||||||||||||
| aux = [] | ||||||||||||||
| # 1) Get native dataset | ||||||||||||||
| if isinstance(ds_or_glob, xr.Dataset): | ||||||||||||||
| ds_native = ds_or_glob | ||||||||||||||
|
|
@@ -220,7 +222,23 @@ def realize_regrid_prepare( | |||||||||||||
| ds_native = open_native_for_cmip_vars( | ||||||||||||||
| [cmip_var], ds_or_glob, mapping, **open_kwargs | ||||||||||||||
| ) | ||||||||||||||
| if not "landfrac" in ds_native and not "ncol" in ds_native: | ||||||||||||||
|
||||||||||||||
| if not "landfrac" in ds_native and not "ncol" in ds_native: | |
| if "landfrac" not in ds_native and "ncol" not in ds_native: |
Outdated
Copilot
AI
Nov 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function should return xr.Dataset according to its signature, but this error path returns a tuple. This type mismatch will cause issues for callers expecting a Dataset. Consider raising an exception instead of returning an error tuple.
| return ( | |
| cmip_var, | |
| f"ERROR: MOM6 grid information is required for variable {cmip_var} " | |
| "but was not provided.", | |
| raise ValueError( | |
| f"MOM6 grid information is required for variable {cmip_var} but was not provided." |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -28,11 +28,19 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _HAS_DASK = False | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Default weight maps; override via function args. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| DEFAULT_CONS_MAP = Path( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "/glade/campaign/cesm/cesmdata/inputdata/cpl/gridmaps/ne30pg3/map_ne30pg3_to_1x1d_aave.nc" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| INPUTDATA_DIR = Path("/glade/campaign/cesm/cesmdata/inputdata/") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| DEFAULT_CONS_MAP_NE30 = Path( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| INPUTDATA_DIR / "cpl/gridmaps/ne30pg3/map_ne30pg3_to_1x1d_aave.nc" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| DEFAULT_BILIN_MAP = Path( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "/glade/campaign/cesm/cesmdata/inputdata/cpl/gridmaps/ne30pg3/map_ne30pg3_to_1x1d_bilin.nc" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| DEFAULT_BILIN_MAP_NE30 = Path( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| INPUTDATA_DIR / "cpl/gridmaps/ne30pg3/map_ne30pg3_to_1x1d_bilin.nc" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) # optional bilinear map | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| DEFAULT_CONS_MAP_T232 = Path( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| INPUTDATA_DIR / "cpl/gridmaps/tx2_3v2/map_t232_TO_1x1d_aave.251023.nc" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| DEFAULT_BILIN_MAP_T232 = Path( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| INPUTDATA_DIR / "cpl/gridmaps/tx2_3v2/map_t232_TO_1x1d_blin.251023.nc" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) # optional bilinear map | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Variables treated as "intensive" → prefer bilinear when available. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -413,10 +421,15 @@ def _pick_maps( | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| conservative_map: Optional[Path] = None, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| bilinear_map: Optional[Path] = None, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| force_method: Optional[str] = None, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| realm: Optional[str] = None, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) -> MapSpec: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """Choose which precomputed map file to use for a variable.""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cons = Path(conservative_map) if conservative_map else DEFAULT_CONS_MAP | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| bilin = Path(bilinear_map) if bilinear_map else DEFAULT_BILIN_MAP | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if realm == "ocn": | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cons = Path(conservative_map) if conservative_map else DEFAULT_CONS_MAP_T232 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| bilin = Path(bilinear_map) if bilinear_map else DEFAULT_BILIN_MAP_T232 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| else: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cons = Path(conservative_map) if conservative_map else DEFAULT_CONS_MAP_NE30 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| bilin = Path(bilinear_map) if bilinear_map else DEFAULT_BILIN_MAP_NE30 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if force_method: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if force_method not in {"conservative", "bilinear"}: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -484,13 +497,19 @@ def regrid_to_1deg_ds( | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Attach time (and bounds) from the original dataset if requested | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if time_from is not None: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ds_out = _attach_time_and_bounds(ds_out, time_from) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if "ncol" in ds_in.dims: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| realm = "atm" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| elif "lndgrid" in ds_in.dims: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| realm = "lnd" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| else: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| realm = "ocn" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Pick the mapfile you used for conservative/bilinear selection | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| spec = _pick_maps( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| varnames[0] if isinstance(varnames, list) else varnames, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| conservative_map=conservative_map, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| bilinear_map=bilinear_map, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| force_method="conservative", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| realm=realm, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) # fx always conservative | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| print(f"using fx map: {spec.path}") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| print(f"using fx map: {spec.path}") | |
| logger.info(f"using fx map: {spec.path}") |
Copilot
AI
Nov 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This condition assumes that any variable without 'ncol' or 'lndgrid' is an ocean variable. This logic is fragile and could fail if new dimension types are added. Consider checking explicitly for expected ocean dimensions ('xh', 'yh') or adding a realm parameter instead of inferring from absence of dimensions.
| if "ncol" not in var_da.dims and "lndgrid" not in var_da.dims: | |
| logger.info("Variable has no 'ncol' or 'lndgrid' dim; assuming ocn variable.") | |
| hdim = "tripolar" | |
| da2 = var_da # Use the DataArray, not the whole Dataset | |
| non_spatial = [d for d in da2.dims if d not in ("yh", "xh")] | |
| realm = "ocn" | |
| method = method or "bilinear" # force bilinear for ocn | |
| else: | |
| da2, non_spatial, hdim = _ensure_ncol_last(var_da) | |
| if hdim == "lndgrid": | |
| da2 = _normalize_land_field(da2, ds_in) | |
| realm = "lnd" | |
| elif hdim == "ncol": | |
| realm = "atm" | |
| if "xh" in var_da.dims and "yh" in var_da.dims: | |
| logger.info("Variable has 'xh' and 'yh' dims; treating as ocean variable.") | |
| hdim = "tripolar" | |
| da2 = var_da # Use the DataArray, not the whole Dataset | |
| non_spatial = [d for d in da2.dims if d not in ("yh", "xh")] | |
| realm = "ocn" | |
| method = method or "bilinear" # force bilinear for ocn | |
| elif "ncol" in var_da.dims or "lndgrid" in var_da.dims: | |
| da2, non_spatial, hdim = _ensure_ncol_last(var_da) | |
| if hdim == "lndgrid": | |
| da2 = _normalize_land_field(da2, ds_in) | |
| realm = "lnd" | |
| elif hdim == "ncol": | |
| realm = "atm" | |
| else: | |
| raise ValueError( | |
| f"Cannot determine grid/realm for variable '{varname}': dims={var_da.dims!r}. " | |
| "Expected 'ncol', 'lndgrid', or both 'xh' and 'yh'." | |
| ) |
Outdated
Copilot
AI
Nov 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This print statement should be replaced with a logger call for consistency with the rest of the codebase changes in this PR.
Outdated
Copilot
AI
Nov 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This print statement should be replaced with a logger call (likely logger.debug() given it's diagnostic information) for consistency with the rest of the codebase changes in this PR.
Outdated
Copilot
AI
Nov 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The regridder is called inside the if hdim == 'lndgrid' block but not before it. For ocean realm processing (lines 640-644), out_norm is never assigned before the denormalization check at line 649, which would cause an UnboundLocalError. The regridder call should be moved before the if hdim == 'lndgrid' check or the logic restructured to handle all realms consistently.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logger is being imported from the
venvmodule, which is incorrect. Thevenvmodule is for creating virtual environments and does not provide a logger. This should likely befrom logging import getLoggeror use the existing logging setup pattern from other files in the project.