Skip to content
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

Implement from_namelist method #43

Merged
merged 28 commits into from
Jul 5, 2021
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
3791e8f
Introduce namelists in the accessor
malmans2 Jun 24, 2021
5375197
Merge branch 'namelists' of https://github.com/malmans2/pyDOMCFG into…
malmans2 Jun 24, 2021
76b1722
Untested, but the main implementation should be in good shape
malmans2 Jun 24, 2021
71d11e5
use class variable introduced
malmans2 Jun 24, 2021
e074650
add tests
malmans2 Jun 24, 2021
749c16a
accessor now handles 999999
malmans2 Jun 25, 2021
b1a3c0e
Update accessor.py
jdha Jun 25, 2021
0da64b5
Merge branch 'pyNEMO:main' into namelists
malmans2 Jun 28, 2021
8ac4c72
introduce _check_namelist_entries
malmans2 Jun 28, 2021
72f0434
typo
malmans2 Jun 28, 2021
4dc3417
tidy up namelist check
malmans2 Jun 28, 2021
82ca98d
add type hints
malmans2 Jun 28, 2021
2a1d06b
minor semplification
malmans2 Jun 29, 2021
c0dfd5c
add more checks
malmans2 Jul 1, 2021
52ac3fb
let Zco handle checks
malmans2 Jul 1, 2021
066adce
deprecate ldbletanh
malmans2 Jul 1, 2021
c68bdf7
fix comment
malmans2 Jul 1, 2021
a9ecfe5
better error print
malmans2 Jul 1, 2021
9f7e188
decorate __call__
malmans2 Jul 2, 2021
54e1b24
clean utils
malmans2 Jul 2, 2021
4b7578d
add tests
malmans2 Jul 2, 2021
2dc3e23
use None
malmans2 Jul 2, 2021
271d7cd
ready for review
malmans2 Jul 5, 2021
501c120
Merge branch 'pyNEMO:main' into namelists
malmans2 Jul 5, 2021
abdb9f2
fix doc and ci
malmans2 Jul 5, 2021
c485512
qqMerge branch 'namelists' of https://github.com/malmans2/pyDOMCFG in…
malmans2 Jul 5, 2021
18b82fa
better docs
malmans2 Jul 5, 2021
2165ab7
avoid numpy with type hint bug
malmans2 Jul 5, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
23 changes: 23 additions & 0 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -68,3 +68,26 @@ jobs:
- name: Run Tests
shell: bash -l {0}
run: pytest --no-cov

bare-environment:
name: bare-environment
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- uses: conda-incubator/setup-miniconda@v2
with:
environment-file: ci/bare-environment.yml
activate-environment: pydomcfg_test_bare
auto-update-conda: false
miniforge-variant: Mambaforge
use-mamba: true

- name: Set up conda environment
shell: bash -l {0}
run: |
python -m pip install -e .
conda list

- name: Run Tests
shell: bash -l {0}
run: pytest --no-cov
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ repos:
hooks:
- id: mypy
exclude: docs
additional_dependencies: [xarray, types-pkg_resources]
additional_dependencies: [xarray, types-pkg_resources, numpy<1.21]
malmans2 marked this conversation as resolved.
Show resolved Hide resolved

- repo: https://github.com/PyCQA/doc8
rev: 0.9.0a1
Expand Down
8 changes: 8 additions & 0 deletions ci/bare-environment.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
name: pydomcfg_test_bare
channels:
- conda-forge
dependencies:
- pytest-cov
- pytest
- pytest-xdist
- xarray
1 change: 1 addition & 0 deletions ci/environment.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,4 @@ dependencies:
- pytest
- pytest-xdist
- xarray
- f90nml
1 change: 1 addition & 0 deletions ci/upstream-dev-env.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,4 @@ dependencies:
- pip
- pip:
- git+https://github.com/pydata/xarray.git
- git+https://github.com/marshallward/f90nml.git
7 changes: 6 additions & 1 deletion docs/users/installing.rst
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@ Required dependencies
- `xarray <http://xarray.pydata.org/>`_
- `numpy <http://www.numpy.org/>`_

Optional dependencies
---------------------

- `f90nml <https://f90nml.readthedocs.io/>`_

Instructions
------------

Expand All @@ -18,5 +23,5 @@ The best way to install all dependencies is to use `conda <http://conda.io/>`_.

.. code-block:: sh

conda install -c conda-forge xarray pip
conda install -c conda-forge xarray f90nml pip
pip install git+https://github.com/pyNEMO/pyDOMCFG.git
112 changes: 109 additions & 3 deletions pydomcfg/accessor.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,21 @@
from typing import Any, Callable, TypeVar, cast
import inspect
import warnings
from collections import ChainMap
from pathlib import Path
from typing import IO, Any, Callable, TypeVar, Union, cast

import xarray as xr
from xarray import Dataset

from .domzgr.zco import Zco
from .utils import _check_namelist_entries

try:
import f90nml

HAS_F90NML = True
except ImportError:
HAS_F90NML = False

F = TypeVar("F", bound=Callable[..., Any])

Expand All @@ -29,7 +41,9 @@ class Accessor:
def __init__(self, xarray_obj: Dataset):
self._obj = xarray_obj
self._jpk = 0
self._nml_ref_path = ""

# Set attributes
@property
def jpk(self) -> int:
"""
Expand All @@ -43,12 +57,104 @@ def jpk(self) -> int:

@jpk.setter
def jpk(self, value: int):
if value <= 0:
raise ValueError("`jpk` MUST be > 0")
if value < 0:
raise ValueError("`jpk` MUST be >= 0 (use 0 to unset jpk)")
self._jpk = value

@property
def nml_ref_path(self) -> str:
"""
Path to reference namelist.

Returns
-------
str
"""
return self._nml_ref_path

@nml_ref_path.setter
def nml_ref_path(self, value: str):
self._nml_ref_path = value

# domzgr methods
@_jpk_check
def zco(self, *args: Any, **kwargs: Any) -> Dataset:
return Zco(self._obj, self._jpk)(*args, **kwargs)

zco.__doc__ = Zco.__call__.__doc__

# Emulate NEMO DOMAINcfg tools
def from_namelist(self, nml_cfg_path_or_io: Union[str, Path, IO[str]]):
"""
TODO
"""

# Parse and return a ChainMap
nml = self._namelist_parser(nml_cfg_path_or_io)

# Pick zgr class
if nml["ln_zco"]:
zgr_class = Zco
else:
raise NotImplementedError("Only `Zco` has been implemented so far.")

# Inspect the __call__ signatures and get the approriate variables
# from the namelists. Replace 999999. with None
parameters = inspect.signature(zgr_class.__call__).parameters
kwargs = {
key: None if nml[key] == 999_999 else nml[key]
for key in parameters
if key != "self"
}

_check_namelist_entries(kwargs)
Copy link
Member Author

@malmans2 malmans2 Jun 28, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can now move _check_namelist_entries in Zgr, so we run the exact same checks to the namelists and the arguments of our classes. (I think we just have to make a small change to support optional arguments).

This is why I'm using isinstance(value, int) which is a less strict check compared to type(value) == int.
For example, isinstance(True, int) is True which is OK for pydomcfg although NEMO would probably complain.
But I think it makes sense to use isinstance as we are not creating namelists to feed into NEMO.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@malmans2 I guess this is where nml_meld will diverge then as 1 != .true. in NEMO speak. But I think that's fine.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, if eventually pydomcfg will make use of nml_meld, I think we just need an extra argument list isinstance=True for pydomcfg but False by default.


# TODO:
# mypy fails here because we convert all 999999 to None,
# even if the argument is not Optional.
# I think we can just switch off the check: The code will eventually fail
# at runtime, but that's because 999999 was used in a wrong place.
# It's the same as running the python version using None with the
# wrong argument.
return zgr_class(self._obj, nml["jpkdta"])(**kwargs) # type: ignore

def _namelist_parser(
self, nml_cfg_path_or_io: Union[str, Path, IO[str]]
) -> ChainMap:

if not HAS_F90NML:
raise ImportError(
"`f90nml` MUST be installed" " to use `obj.domcfg.from_namelist()`"
)

if not self.nml_ref_path:
raise ValueError(
"Set `nml_ref_path` before calling `obj.domcfg.from_namelist()`"
" For example: obj.domcfg.nml_ref_path = 'path/to/nml_ref'"
)

if self.jpk:
warnings.warn(
"`obj.domcfg.jpk` is ignored." " `jpk` is inferred from the namelists."
)

# Read namelists: cfg overrides ref
# TODO:
# If we specify the third argument, the resulting namelist is written
# The advantage is that it is formatted as the reference namelist
# We could do it to a tmp file, then add the resulting string to the
# global attributes of the final dataset.
nml_cfg = f90nml.read(nml_cfg_path_or_io)
nml = f90nml.patch(self.nml_ref_path, nml_cfg)

# Chain all namblocks
chained = ChainMap(*nml.todict().values())

mutually_exclusive = ("ln_zco", "ln_zps", "ln_sco")
if sum(chained.get(key) for key in mutually_exclusive) != 1:
raise ValueError(
"One and only one of the following variables MUST be set:"
f" {mutually_exclusive}"
)

return chained
10 changes: 4 additions & 6 deletions pydomcfg/domzgr/zco.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@
import numpy as np
from xarray import DataArray, Dataset

from pydomcfg.utils import _are_nemo_none, _is_nemo_none

from .zgr import Zgr


Expand Down Expand Up @@ -147,7 +145,7 @@ def _compute_pp(

# Uniform grid, return dummy zeros
if self._is_uniform:
if not all(_are_nemo_none((ppsur, ppa0, ppa1))):
if not all(pp is None for pp in (ppsur, ppa0, ppa1)):
warnings.warn(
"Uniform grid case (no stretching):"
" ppsur, ppa0 and ppa1 are ignored when ppacr == ppkth == 0"
Expand All @@ -163,10 +161,10 @@ def _compute_pp(
ee = np.log(np.cosh((1.0 - self._ppkth) / self._ppacr))

# Substitute only if is None or 999999
ppa1_out = (aa / (bb - cc * (dd - ee))) if _is_nemo_none(ppa1) else ppa1
ppa0_out = (self._ppdzmin - ppa1_out * bb) if _is_nemo_none(ppa0) else ppa0
ppa1_out = (aa / (bb - cc * (dd - ee))) if ppa1 is None else ppa1
ppa0_out = (self._ppdzmin - ppa1_out * bb) if ppa0 is None else ppa0
ppsur_out = (
-(ppa0_out + ppa1_out * self._ppacr * ee) if _is_nemo_none(ppsur) else ppsur
-(ppa0_out + ppa1_out * self._ppacr * ee) if ppsur is None else ppsur
)

return (ppsur_out, ppa0_out, ppa1_out)
Expand Down
61 changes: 61 additions & 0 deletions pydomcfg/tests/data.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,3 +43,64 @@
[5250.2266, 5000.0000, 500.5646, 500.3288],
]
)

# See pag 62 of v3.6 manual for the input parameters
# TODO:
# ppdzmin and pphmax in NEMO DOMAINcfg README are actually 999999,
# a dummy value is assigned but it's a temporary workaround.
# See: https://github.com/pyNEMO/pyDOMCFG/issues/44
ORCA2_NAMELIST = """
!-----------------------------------------------------------------------
&namcfg ! parameters of the configuration
!-----------------------------------------------------------------------
!
ln_e3_dep = .false. ! =T : e3=dk[depth] in discret sens.
! ! ===>>> will become the only possibility in v4.0
! ! =F : e3 analytical derivative of depth function
! ! only there for backward compatibility test with v3.6
! !
cp_cfg = "orca" ! name of the configuration
jp_cfg = 2 ! resolution of the configuration
jpidta = 180 ! 1st lateral dimension ( >= jpi )
jpjdta = 148 ! 2nd " " ( >= jpj )
jpkdta = 31 ! number of levels ( >= jpk )
Ni0glo = 180 ! 1st dimension of global domain --> i =jpidta
Nj0glo = 148 ! 2nd - - --> j =jpjdta
jpkglo = 31
jperio = 4 ! lateral cond. type (between 0 and 6)
ln_use_jattr = .false. ! use (T) the file attribute: open_ocean_jstart, if present
! in netcdf input files, as the start j-row for reading
ln_domclo = .false. ! computation of closed sea masks (see namclo)
/
!-----------------------------------------------------------------------
&namdom !
!-----------------------------------------------------------------------
jphgr_msh = 0 ! type of horizontal mesh
ppglam0 = 999999.0 ! longitude of first raw and column T-point (jphgr_msh = 1)
ppgphi0 = 999999.0 ! latitude of first raw and column T-point (jphgr_msh = 1)
ppe1_deg = 999999.0 ! zonal grid-spacing (degrees)
ppe2_deg = 999999.0 ! meridional grid-spacing (degrees)
ppe1_m = 999999.0 ! zonal grid-spacing (degrees)
ppe2_m = 999999.0 ! meridional grid-spacing (degrees)
ppsur = -4762.96143546300 ! ORCA r4, r2 and r05 coefficients
ppa0 = 255.58049070440 ! (default coefficients)
ppa1 = 245.58132232490 !
ppkth = 21.43336197938 !
ppacr = 3.0 !
ppdzmin = 10.0 ! Minimum vertical spacing
pphmax = 5000.0 ! Maximum depth
ldbletanh = .FALSE. ! Use/do not use double tanf function for vertical coordinates
ppa2 = 999999.0 ! Double tanh function parameters
ppkth2 = 999999.0 !
ppacr2 = 999999.0 !
/
!-----------------------------------------------------------------------
&namzgr ! vertical coordinate (default: NO selection)
!-----------------------------------------------------------------------
ln_zco = .true. ! z-coordinate - full steps
ln_zps = .false. ! z-coordinate - partial steps
ln_sco = .false. ! s- or hybrid z-s-coordinate
ln_isfcav = .false. ! ice shelf cavity
ln_linssh = .false. ! linear free surface
/
"""
2 changes: 1 addition & 1 deletion pydomcfg/tests/test_accessor.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ def test_jpk():
ds_bathy.domcfg.zco()

# jpk must be > 0
with pytest.raises(ValueError, match="`jpk` MUST be > 0"):
with pytest.raises(ValueError, match="`jpk` MUST be >= 0"):
ds_bathy.domcfg.jpk = -1

# Has been set correctly.
Expand Down