refactor: storage, layout, and devices (mostly cartesian) #1880
Assignees
Labels
gt4py.cartesian
Issues concerning the current version with support only for cartesian grids.
gt4py.next
Issues concerning the new version with support for non-cartesian grids.
module: storage
Related to storage subpackage
priority: low
Low priority task
Comments
romanc
added
gt4py.cartesian
2 tasks
2 tasks
romanc
added a commit
that referenced
this issue
Mar 4, 2025
## Description Following the [YAGNI principle](https://github.com/GridTools/gt4py/blob/main/CODING_GUIDELINES.md#software-design), stick to the device types that are actually supported in the codebase. We can add support for other devices later. @havogt as discussed the other day, let's have a discussion about whether or not we need all these device types defined. I know that @FlorianDeconinck would like to work towards over protection and throwing errors rather sooner than later for unsupported things (very much in general). This is part of a clean-up series tracked in issue #1880. ## Requirements - [ ] All fixes and/or new features come with corresponding tests. N/A - [ ] Important design decisions have been documented in the appropriate ADR inside the [docs/development/ADRs/](docs/development/ADRs/Index.md) folder. N/A Co-authored-by: Roman Cattaneo <[email protected]>
2 tasks
romanc
added a commit
that referenced
this issue
Mar 17, 2025
## Description This PR centralizes the definition of `CUPY_DEVICE_TYPE` in `_core/definitions`. This effectively de-duplicates the definition of `CUPY_DEVICE` in gt4py next and cartesian. Fixes a couple of typos along the way. Related issue: #1880 ## Requirements - [x] All fixes and/or new features come with corresponding tests. Should be covered by existing tests. - [ ] Important design decisions have been documented in the appropriate ADR inside the [docs/development/ADRs/](docs/development/ADRs/Index.md) folder. N/A --------- Co-authored-by: Roman Cattaneo <[email protected]>
2 tasks
romanc
added a commit
that referenced
this issue
Mar 19, 2025
…USE_HIP` (#1916) ## Description This PR breaks the dependency cycle between `gt4py.cartesian` and `gt4py.storage`. The last puzzle piece was the distinction between AMD and NVIDIA GPUs. This was controlled by `GT4PY_USE_HIP`. With this PR we re-use `CUPY_DEVICE_TYPE` (for `_core/definitions`) for this purpose. `GT4PY_USE_HIP` could be set by an environment variable or be auto-detected. Auto-detection for `GT4PY_USE_HIP` and `CUPY_DEVICE_TYPE` is the same. And according to @stubbiali, the environment variable was never[^1] needed because auto-detection worked well. We thus don't expect issues removing the environment variable. Related issue: #1880 ## Requirements - [x] All fixes and/or new features come with corresponding tests. Assumed to covered by existing tests. - [ ] Important design decisions have been documented in the appropriate ADR inside the [docs/development/ADRs/](docs/development/ADRs/Index.md) folder. N/A [^1]: #1867 (comment) Co-authored-by: Roman Cattaneo <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Description
PRs #1843 and #1867 raised a couple of questions regarding storage, layouts, and device support/handling on the cartesian side. This is a collector issue in which I'd like to group a couple small PRs and maybe have discussions around these refactors.
So far, we've identified a potential for improvement in the following areas:
DeviceTypeto what is actually used/supported.PR refactor: stick to supported device types #1879
CUPY_DEVICE(currently defined separately innextandcartesian) in_corePR refactor: centralize
CUPY_DEVICE_TYPEin_core/definitions#1908storageandcartesian, see this commentPR refactor[cartesian, storage]: break dependency cycle; replace
GT4PY_USE_HIP#1916GT4PY_USE_HIPenvironment variable since auto-detection of NVIDIA/AMD GPUs seems to work fine, see this commentPR refactor[cartesian, storage]: break dependency cycle; replace
GT4PY_USE_HIP#1916LayoutintoBackend, see refactor[cartesian]: useDeviceTypefrom core_defn inLayoutInfo#1867 (comment)Consider this as a low-priority wish list for refactors on the side.
The text was updated successfully, but these errors were encountered: