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

add time clock to runtime #170

Merged
merged 41 commits into from
Mar 8, 2021
Merged
Show file tree
Hide file tree
Changes from 38 commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
2955bca
added a main_clock with dimension 'mclock' to initialize context, thi…
Feb 8, 2021
770d1ec
fixed black
Feb 8, 2021
7b0326e
changed 'master' to 'main' everywhere except in non-breaking testcases.
Feb 8, 2021
7384b47
fixed space and undid changes of access-clock
Feb 8, 2021
d8165be
added placeholder
Feb 8, 2021
d2a3030
fixed black (again)
Feb 8, 2021
bc26be9
added master_clock_dim + tests, master_clock_coords no tests
Feb 9, 2021
95a4b4d
Update xsimlab/xr_accessor.py
feefladder Feb 9, 2021
76033e8
Update xsimlab/xr_accessor.py
feefladder Feb 9, 2021
2214139
Update xsimlab/xr_accessor.py
feefladder Feb 9, 2021
570489e
added master/main_clock_coord access tests, updated whats-new
Feb 9, 2021
800f2b8
Merge branch 'master-main' of github.com:Joeperdefloep/xarray-simlab …
Feb 9, 2021
6f38346
removed vscode settings
Feb 9, 2021
13f0045
created duck-typed singleton class for main clock and managed access
Feb 9, 2021
6aa4f37
stuck at 'store_output_vars' has unmatching dimensions
Feb 9, 2021
5f1f6a8
Apply suggestions from code review
feefladder Feb 10, 2021
6c5a223
removed raise checks from test_update_clocks_master_clock_warning
Feb 10, 2021
605594d
Merge branch 'master-main' of github.com:Joeperdefloep/xarray-simlab …
Feb 10, 2021
a9dabac
removed raises in test_update_clocks_master_warning
Feb 10, 2021
1589466
removed redundancy in test_update_master_clock
Feb 10, 2021
cceed31
properly implemented singleton, should work now
Feb 10, 2021
10673e9
added tests
Feb 10, 2021
1d11793
added check for double dimensions.
Feb 10, 2021
60127a7
Merge branch 'access-clock' into master-main
feefladder Feb 10, 2021
5e0f75c
Merge pull request #1 from Joeperdefloep/master-main
feefladder Feb 10, 2021
404cddd
Merge branch 'master' into access-clock
feefladder Feb 10, 2021
482c1f9
stuff, deleted prints
Feb 10, 2021
16330bd
Update xsimlab/utils.py
feefladder Feb 11, 2021
0abba09
Update xsimlab/variable.py
feefladder Feb 11, 2021
2b05f12
better tests
Feb 11, 2021
9be2a50
Merge branch 'access-clock' of github.com:Joeperdefloep/xarray-simlab…
Feb 11, 2021
90b0463
black...
Feb 11, 2021
097a9aa
stupid test solved
Feb 11, 2021
a0d80c0
Update xsimlab/xr_accessor.py
feefladder Feb 19, 2021
5bc5b6e
Update xsimlab/tests/test_model.py
feefladder Feb 19, 2021
d8a1fa9
did some stuff
Feb 19, 2021
c9139ca
Merge branch 'access-clock' of github.com:Joeperdefloep/xarray-simlab…
Feb 19, 2021
5f84d67
should be good
Feb 19, 2021
08a17e8
docs
Mar 2, 2021
fc4d3dd
updated what's new
Mar 4, 2021
6d9ffa7
Merge branch 'master' into access-clock
benbovy Mar 8, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
Copy link
Member

Choose a reason for hiding this comment

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

Could you remove this file, please? It should be ignored in the next PRs (#169).

"python.pythonPath": "/home/joempie/miniconda3/envs/simlab-dev/bin/python"
}
2 changes: 1 addition & 1 deletion doc/api.rst
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ Variable

.. autosummary::
:toctree: _api_generated/

MAIN_CLOCK
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, adding MAIN_CLOCK here yields an error when building the docs with Sphinx (as a result the whole Variable section is empty).

Copy link
Contributor Author

@feefladder feefladder Mar 2, 2021

Choose a reason for hiding this comment

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

hmm I've checked that, and I don't get an error. That is, I did get a "could not import module MAIN_CLOCK" or something, when I didn't first python -m pip install ., but after that it built properly.
e.g. make html worked properly without errors.
link

variable
index
any_object
Expand Down
1 change: 1 addition & 0 deletions xsimlab/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
group,
group_dict,
)
from .utils import MAIN_CLOCK
from .xr_accessor import SimlabAccessor, create_setup
from . import monitoring

Expand Down
8 changes: 7 additions & 1 deletion xsimlab/drivers.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from .hook import flatten_hooks, group_hooks, RuntimeHook
from .process import RuntimeSignal
from .stores import ZarrSimulationStore
from .utils import get_batch_size
from .utils import get_batch_size, MAIN_CLOCK


class ValidateOption(Enum):
Expand All @@ -28,6 +28,8 @@ class RuntimeContext(Mapping[str, Any]):
"batch",
"sim_start",
"sim_end",
"main_clock_values",
"main_clock_dataarray",
"step",
"nsteps",
"step_start",
Expand Down Expand Up @@ -161,6 +163,8 @@ def _generate_runtime_datasets(dataset):
init_data_vars = {
"_sim_start": mclock_coord[0],
"_nsteps": dataset.xsimlab.nsteps,
# since we pass a dataset, we need to set the coords
"_main_clock_values": dataset.coords[mclock_dim].data,
"_sim_end": mclock_coord[-1],
}

Expand Down Expand Up @@ -327,6 +331,8 @@ def _run(
sim_start=ds_init["_sim_start"].values,
nsteps=ds_init["_nsteps"].values,
sim_end=ds_init["_sim_end"].values,
main_clock_values=ds_init["_main_clock_values"].values,
main_clock_dataarray=dataset.xsimlab.main_clock_coord,
)

in_vars = _get_input_vars(ds_init, model)
Expand Down
11 changes: 9 additions & 2 deletions xsimlab/stores.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import zarr

from . import Model
from .utils import get_batch_size, normalize_encoding
from .utils import get_batch_size, normalize_encoding, MAIN_CLOCK
from .variable import VarType


Expand Down Expand Up @@ -241,6 +241,14 @@ def _create_zarr_dataset(
f"its accepted dimension(s): {var_info['metadata']['dims']}"
)

# set MAIN_CLOCK placeholder to main_clock dimension
if self.mclock_dim in dim_labels and MAIN_CLOCK in dim_labels:
Copy link
Member

Choose a reason for hiding this comment

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

If we don't allow declaring input variables with xs.MAIN_CLOCK, this case shouldn't happen.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This can still happen: when a user declares a variabke with dims
`(xs.MAIN_CLOCK,'clock')'

Copy link
Member

Choose a reason for hiding this comment

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

Ah yes! Hmm then we would probably want to check in Dataset.xsimlab.update_clocks that the given clock labels (main clock and other clocks if provided) don't conflict with the dimension labels set in the model. I don't think we do this already.

For convenience we could add a property in xs.Model that returns a list of all (unique) dimension labels declared in the model.

Let's fix that in another PR..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not sure if that is the right place though: Am I right that a user may add a process after updating clocks, thus not catching it?

raise ValueError(
f"Main clock: '{self.mclock_dim}' has a duplicate in {dim_labels}."
"Please change the name of 'main_clock' in `create_setup`"
)
dim_labels = [self.mclock_dim if d is MAIN_CLOCK else d for d in dim_labels]

if clock is not None:
dim_labels.insert(0, clock)
if add_batch_dim:
Expand Down Expand Up @@ -320,7 +328,6 @@ def write_output_vars(self, batch: int, step: int, model: Optional[Model] = None

else:
idx_dims = [clock_inc] + [slice(0, n) for n in np.shape(value)]

if batch != -1:
idx_dims.insert(0, batch)

Expand Down
65 changes: 65 additions & 0 deletions xsimlab/tests/test_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -452,3 +452,68 @@ def initialize(self):
model.execute("initialize", {})

assert model.state[("baz", "actual")] == Frozen({("foo", "a"): 1, ("bar", "b"): 2})


def test_main_clock_access():
@xs.process
class Foo:
a = xs.variable(intent="out", dims=xs.MAIN_CLOCK)
b = xs.variable(intent="out", dims=xs.MAIN_CLOCK)

@xs.runtime(args=["main_clock_values", "main_clock_dataarray"])
def initialize(self, clock_values, clock_array):
self.a = clock_values * 2
np.testing.assert_equal(self.a, [0, 2, 4, 6])
self.b = clock_array * 2
assert clock_array.dims[0] == "clock"
assert all(clock_array[clock_array.dims[0]].data == [0, 1, 2, 3])

@xs.runtime(args=["step_delta", "step"])
def run_step(self, dt, n):
Copy link
Member

Choose a reason for hiding this comment

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

You could remove dt if you don't use it.

assert self.a[n] == 2 * n
self.a[n] += 1

model = xs.Model({"foo": Foo})
ds_in = xs.create_setup(
model=model,
clocks={"clock": range(4)},
input_vars={},
output_vars={"foo__a": None},
)
ds_out = ds_in.xsimlab.run(model=model)
assert all(ds_out.foo__a.data == [1, 3, 5, 6])

# test for error when another dim has the same name as xs.MAIN_CLOCK
@xs.process
class DoubleMainClockDim:
a = xs.variable(intent="out", dims=("clock", xs.MAIN_CLOCK))

def initialize(self):
self.a = [[1, 2, 3], [3, 4, 5]]

def run_step(self):
self.a += self.a

model = xs.Model({"foo": DoubleMainClockDim})
with pytest.raises(ValueError, match=r"Main clock:*"):
xs.create_setup(
model=model,
clocks={"clock": [0, 1, 2, 3]},
input_vars={},
output_vars={"foo__a": None},
).xsimlab.run(model)

# test for error when trying to put xs.MAIN_CLOCK as a dim in an input var
with pytest.raises(
ValueError, match="Do not pass xs.MAIN_CLOCK into input vars dimensions"
):
a = xs.variable(intent="in", dims=xs.MAIN_CLOCK)

with pytest.raises(
ValueError, match="Do not pass xs.MAIN_CLOCK into input vars dimensions"
):
b = xs.variable(intent="in", dims=(xs.MAIN_CLOCK,))
with pytest.raises(
ValueError, match="Do not pass xs.MAIN_CLOCK into input vars dimensions"
):
c = xs.variable(intent="in", dims=["a", ("a", xs.MAIN_CLOCK)])
4 changes: 2 additions & 2 deletions xsimlab/tests/test_xr_accessor.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,9 +158,9 @@ def test_master_clock_coords_warning(self):
)
with pytest.warns(
FutureWarning,
match="master_clock is to be deprecated in favour of main_clock",
match="master_clock_coord is to be deprecated in favour of main_clock",
):
xr.testing.assert_equal(ds.xsimlab.master_clock_coord, ds.mclock)
ds.xsimlab.master_clock_coord

def test_clock_sizes(self):
ds = xr.Dataset(
Expand Down
28 changes: 28 additions & 0 deletions xsimlab/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,34 @@
V = TypeVar("V")


class _MainClockDim:
"""Singleton class to be used as a placeholder of the main clock
dimension.

It will be replaced by the actual dimension label set during simulation setup
(i.e., ``main_clock`` argument).

"""

_singleton = None

def __new__(cls):
if _MainClockDim._singleton is None:
# if there is no instance of it yet, create a class instance
_MainClockDim._singleton = super(_MainClockDim, cls).__new__(cls)
return _MainClockDim._singleton

def __repr__(self):
return "MAIN_CLOCK (undefined)"


MAIN_CLOCK = _MainClockDim()
Copy link
Member

Choose a reason for hiding this comment

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

It would be nice to add some docstrings for this attribute, e.g.,

MAIN_CLOCK = _MainClockDim()
"""
Sentinel to indicate simulation's main clock dimension, to be
replaced by the actual dimension label set in input/output datasets.
"""

and add it in the docs (probably in API references in the variables section, since we would use it when declaring variables).

"""
Sentinel to indicate simulation's main clock dimension, to be
replaced by the actual dimension label set in input/output datasets.
"""


def variables_dict(process_cls):
"""Get all xsimlab variables declared in a process.

Expand Down
15 changes: 12 additions & 3 deletions xsimlab/variable.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import attr
from attr._make import _CountingAttr

from .utils import normalize_encoding
from .utils import normalize_encoding, MAIN_CLOCK


class VarType(Enum):
Expand Down Expand Up @@ -57,12 +57,18 @@ def _as_dim_tuple(dims):
ambiguous and thus not allowed.

"""
if not len(dims):
# MAIN_CLOCK is sentinel and does not have length (or zero), so check explicitly
if dims is MAIN_CLOCK:
dims = [(dims,)]
elif not len(dims):
dims = [()]
elif isinstance(dims, str):
dims = [(dims,)]
elif isinstance(dims, list):
dims = [tuple([d]) if isinstance(d, str) else tuple(d) for d in dims]
dims = [
tuple([d]) if (isinstance(d, str) or d is MAIN_CLOCK) else tuple(d)
for d in dims
]
else:
dims = [dims]

Expand Down Expand Up @@ -221,6 +227,9 @@ def variable(
else:
_init = True
_repr = True
# also check if MAIN_CLOCK is there
if any([MAIN_CLOCK in d for d in metadata["dims"]]):
raise ValueError("Do not pass xs.MAIN_CLOCK into input vars dimensions")

return attr.attrib(
metadata=metadata,
Expand Down
5 changes: 3 additions & 2 deletions xsimlab/xr_accessor.py
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ def master_clock_coord(self):
Returns None if no main clock is defined in the dataset.
"""
warnings.warn(
"master_clock is to be deprecated in favour of main_clock",
"master_clock_coord is to be deprecated in favour of main_clock",
FutureWarning,
)
return self.main_clock_coord
Expand All @@ -224,7 +224,7 @@ def main_clock_coord(self):

@property
def nsteps(self):
"""Number of simulation steps, computed from the master
"""Number of simulation steps, computed from the main
clock coordinate.

Returns 0 if no main clock is defined in the dataset.
Expand Down Expand Up @@ -319,6 +319,7 @@ def _uniformize_clock_coords(self, dim=None, units=None, calendar=None):
)

def _set_input_vars(self, model, input_vars):

invalid_inputs = set(input_vars) - set(model.input_vars)

if invalid_inputs:
Expand Down