Remove spacedomain vertical dimension #69
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 29 commits
November 3, 2021 16:46
The new constraint is that: "each component temporal resolution must be an integer multiple of the other components' resolutions". So the time domain compatibility check in `Clock` is revised accordingly. Also tests are added in the docstring to make sure that the check functions and remains functioning as expected.
The new constraint is that: "each component temporal resolution must be an integer multiple of the other components' resolutions". So a space domain compatibility check in `Compass` is revised accordingly. The `Grid` is fitted with a new method `is_matched_in` to determine whether one grid is "contained" in another. Also tests are added in the docstring to make sure that the check functions and remains functioning as expected. Also new Grid method `is_matched_in` is added to the API reference.
This method is used in `Compass` without a check on type so this is a method that needs to be implemented for all `SpaceDomain`s.
…aints
The component temporal resolutions used to be {SL: daily, SS: 3daily, OW: 2daily}, but 3 is not a multiple of 2. They are now {SL: daily, SS: 4daily, OW:2daily}. The simulation period is also extended to 16 days, so that each spin up period is half of the simulation period.
The component spatial resolutions used to be {SL: 1deg, SS: 0.5deg, OW: 0.2deg}, but 0.5 is not a multiple of 0.2. They are now {SL: 1deg, SS: 0.5deg, OW:0.25deg}.
The expected records needed to be modified to take into account these modifications. These were manually calculated, and then compared against the results using the framework to make sure these new values are robust - they are essential because they form the basis for checking that the framework has not been broken.
The input files and the substitute data files needed to also be modified to comply with the resolutions and temporal extent changes.
The YAML configuration files are also modified to reflect the changes in the tests themselves.
Since `cfdm` and `cf-python` have dropped support for python 3.6, we need to follow. End of support for it is near anyway (2021-12-23 https://www.python.org/downloads/).
and implement them for `Grid`. The method `has_vertical_axis` is to distinguish 3D from 2D space domains. The property `vertical_axis` is to retrieve the axis name for the vertical dimension (e.g. 'Z' in a `Grid` context).
since the geometrical intersection between two volumes is a surface, the vertical dimension of the `SpaceDomain` should not be involved
since these 3 attributes of a space domain are physically only meaningful horizontally, even if `SpaceDomain` has a vertical axis. So the compatibility check between the `SpaceDomain` and the data field is done on the horizontal `Grid` only, the data field is squeezed along Z, and the Z axis is dropped in the space shape of the processed data.
otherwise it would raise a quiet uninformative IndexError
this should have been included in the commit for unifhy-org#61
this avoids having an annoying warning message when compiling
While the vertical axis in `Grid` is perfectly functional, it should not be exposed through the API until 3D components are effectively supported (i.e. until contributors are able to tell the framework if they are expecting 2D fields or 3D fields or both). Rather than deleting the functionality, comments have been used wherever needed to prevent users from creating a 3D `Grid`. That involves: - removing the parameters in instantiation methods, - removing references to these parameters in docstrings, and - removing Z related properties from API reference (even though these still exist, and in fact are still accessible but will return `None`).
ThibHlln
changed the title
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
resolve #28
Currently, depending on the user choise on defining/not defining a vertical dimension in their
SpaceDomain, 2D or 3D arrays are sent to the components, which is problematic from a component contributor perspective because they don't know a priori what is the shape of the domain they need to work on, and they may not even want to work with the vertical dimension at all. There needs to be a way for the contributor to specify the space domains they support, and until then, this 2D/3D mismatch needs to be prevented by limiting to 2D domains.Moreover, inherently horizontal properties of
SpaceDomain(i.e.land_sea_mask,flow_direction, andcell_area) are not limited to 2D-only shapes. This also needs to be changed.This PR includes:
Gridand its derived classes so that users cannot define 3D domains but only 2D domainsland_sea_mask,flow_direction, andcell_areaExchangerNote, while the user cannot define 3D grids anymore, the functionality to do so, which was perfectly functional, is kept in the source code, only vertical dimension related "entry points" are deactivated (i.e. commented out) to prevent the use of this functionality. This way, once 3D domains are properly supported, this can be brought back easily.