-
Notifications
You must be signed in to change notification settings - Fork 16
Store information about the halo-size in the allocated Quantity #265
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
Changes from all commits
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 | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -36,22 +36,29 @@ def __init__( | |||||||||||
| extent: Sequence[int] | None = None, | ||||||||||||
| gt4py_backend: str | None = None, | ||||||||||||
| allow_mismatch_float_precision: bool = False, | ||||||||||||
| number_of_halo_points: int = 0, | ||||||||||||
| ): | ||||||||||||
| """ | ||||||||||||
| Initialize a Quantity. | ||||||||||||
| """Initialize a Quantity. | ||||||||||||
|
|
||||||||||||
| Args: | ||||||||||||
| data: ndarray-like object containing the underlying data | ||||||||||||
| dims: dimension names for each axis | ||||||||||||
| units: units of the quantity | ||||||||||||
| origin: first point in data within the computational domain | ||||||||||||
| extent: number of points along each axis within the computational domain | ||||||||||||
| gt4py_backend: backend to use for gt4py storages, if not given this will | ||||||||||||
| be derived from a Storage if given as the data argument, otherwise the | ||||||||||||
| storage attribute is disabled and will raise an exception. Will raise | ||||||||||||
| a TypeError if this is given with a gt4py storage type as data | ||||||||||||
| """ | ||||||||||||
| data (_type_): ndarray-like object containing the underlying data | ||||||||||||
| dims (Sequence[str]): dimension names for each axis | ||||||||||||
| units (str): units of the quantity | ||||||||||||
| origin (Sequence[int] | None, optional): first point in data within the | ||||||||||||
| computational domain. Defaults to None. | ||||||||||||
| extent (Sequence[int] | None, optional): number of points along each axis | ||||||||||||
| within the computational domain. Defaults to None. | ||||||||||||
| gt4py_backend (str | None, optional): backend to use for gt4py storages, | ||||||||||||
| if not given this will be derived from a Storage | ||||||||||||
| if given as the data argument. Defaults to None. | ||||||||||||
| allow_mismatch_float_precision (bool, optional): allow for precision that is | ||||||||||||
| not the simulation-wide default configuration. Defaults to False. | ||||||||||||
| number_of_halo_points (int, optional): Number of halo points used. Defaults to 0. | ||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not so sure about the name here because it's not really the number of halo points, right? If I put 3, there will be more then 3 halo points. Though not optimal either, I'd stick with what we already have (
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. well,
or NDSL/ndsl/initialization/grid_sizer.py Line 14 in c180927
or Line 44 in c180927
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. then let's change all of them because they are equally wrong in that sense?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is the lingo that is used to describe the halo size. This is not changeable
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't our job here to dare new things and sometimes break with (old) conventions? Like, we tell atmospheric scientists that they can now write code in python(-ish) and that they can have variable names longer than 5 characters? We even make use of the term "halo size" in our introduction examples when referring to the NDSL/examples/NDSL/02_NDSL_basics.ipynb Line 19 in c180927
|
||||||||||||
|
|
||||||||||||
| Raises: | ||||||||||||
| ValueError: Data-type mismatch between configuration and input-data | ||||||||||||
| TypeError: Typing of the data that does not fit | ||||||||||||
| """ | ||||||||||||
| if ( | ||||||||||||
| not allow_mismatch_float_precision | ||||||||||||
| and is_float(data.dtype) | ||||||||||||
|
|
@@ -108,15 +115,14 @@ def __init__( | |||||||||||
| ) | ||||||||||||
| ) | ||||||||||||
| else: | ||||||||||||
| if data is None: | ||||||||||||
| raise TypeError("requires 'data' to be passed") | ||||||||||||
| # We have no info about the gt4py_backend, so just assign it. | ||||||||||||
| self._data = data | ||||||||||||
|
|
||||||||||||
| _validate_quantity_property_lengths(data.shape, dims, origin, extent) | ||||||||||||
| self._metadata = QuantityMetadata( | ||||||||||||
| origin=_ensure_int_tuple(origin, "origin"), | ||||||||||||
| extent=_ensure_int_tuple(extent, "extent"), | ||||||||||||
| n_halo=number_of_halo_points, | ||||||||||||
| dims=tuple(dims), | ||||||||||||
| units=units, | ||||||||||||
| data_type=type(self._data), | ||||||||||||
|
|
@@ -135,6 +141,7 @@ def from_data_array( | |||||||||||
| origin: Sequence[int] | None = None, | ||||||||||||
| extent: Sequence[int] | None = None, | ||||||||||||
| gt4py_backend: str | None = None, | ||||||||||||
| number_of_halo_points: int = 0, | ||||||||||||
| ) -> Quantity: | ||||||||||||
| """ | ||||||||||||
| Initialize a Quantity from an xarray.DataArray. | ||||||||||||
|
|
@@ -155,6 +162,7 @@ def from_data_array( | |||||||||||
| data_array.attrs["units"], | ||||||||||||
| origin=origin, | ||||||||||||
| extent=extent, | ||||||||||||
| number_of_halo_points=number_of_halo_points, | ||||||||||||
| gt4py_backend=gt4py_backend, | ||||||||||||
| ) | ||||||||||||
|
|
||||||||||||
|
|
@@ -172,6 +180,13 @@ def to_netcdf( | |||||||||||
| ) | ||||||||||||
|
|
||||||||||||
| def halo_spec(self, n_halo: int) -> QuantityHaloSpec: | ||||||||||||
| # This is a preliminary check to see if this is ever triggered. | ||||||||||||
| # If not, we can remove it down the line and change the call signature. | ||||||||||||
| if n_halo != self._metadata.n_halo: | ||||||||||||
| warnings.warn( | ||||||||||||
| "Found inconsistency with number of halo points in Quantity:" | ||||||||||||
| + f"{n_halo} vs {self._metadata.n_halo}" | ||||||||||||
| ) | ||||||||||||
| return QuantityHaloSpec( | ||||||||||||
| n_halo, | ||||||||||||
| self.data.strides, | ||||||||||||
|
|
||||||||||||
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.
Maybe "halo size" rather than "halo points"?
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.
see above