Edits to support icon-dsl on Daint #693
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
|
jonasjucker
reviewed
Mar 27, 2023
| @@ -131,7 +135,7 @@ class Icon(AutotoolsPackage): | |||
| description= | |||
| 'Enable extension of eccodes with center specific definition files') | |||
|
|
|||
| depends_on('py-icon4py@0.0.1%gcc') | |||
| depends_on('py-icon4py@main%gcc') | |||
There was a problem hiding this comment.
Better remove the @version completely and set it explicitely in an spack.yaml. This recipe needs to be as general as possible. Otherwise we can only build one specific version of icon-dsl
jonasjucker
reviewed
Mar 27, 2023
| '-ccbin {0}'.format(cuda_host_compiler), '-g', '-O3', | ||
| '-arch=sm_{0}'.format(gpu) | ||
| ]) | ||
| config_vars['NVCFLAGS'].extend( |
There was a problem hiding this comment.
This breaks the variant +cuda-gcc since it relies on cuda_host_compiler.
Please find a way to not break it.
jonasjucker
reviewed
Mar 27, 2023
| @@ -55,7 +55,9 @@ def install(self, spec, prefix): | |||
| args.append('.') | |||
|
|
|||
| pip = inspect.getmodule(self).pip | |||
| build_dirs = ['common', 'pyutils', 'testutils', 'atm_dyn_iconam'] | |||
| build_dirs = [ | |||
There was a problem hiding this comment.
Here again you break the recipe for older version of icon4py.
I see two solutions:
Either we make each folder a variant, i.e. +liskov +common, ... or you restrict liskov to a newer version of icon4py.
jonasjucker
approved these changes
Mar 27, 2023
There was a problem hiding this comment.
I made some suggestions.
The most important thing, making the dsl build optional trough a variant +dsl is missing.
This PR only merges into another feature-branch, so you don't have to adress my suggestions now. But eventually the need to be solved before merging into main.
|
thanks! I'll merge now and then properly address your comments in a later PR |
abishekg7
added a commit
that referenced
this pull request
Apr 13, 2023
* add versions for icon4py and gt4py * GitHub Action: Apply Pep8-formatting * add fix for py-poetry-core * GitHub Action: Apply Pep8-formatting * updates to get icon4py building after the major gt4py overhaul * Update system_test.py * GitHub Action: Apply Pep8-formatting * add missing test deps, only test next and eve * remove obsolete patch * addressing some review comments * Add Boost to CMake Include Path * Change icon, gridtools and balfrin site packages for successful build * GitHub Action: Apply Pep8-formatting * set boost-path for unittests of icon4py, make tests on Balfrin work * lock python and gt4py versions * GitHub Action: Apply Pep8-formatting * fix typo * GitHub Action: Apply Pep8-formatting * Edits to support icon-dsl on Daint (#693) * working on daint * GitHub Action: Apply Pep8-formatting * tested for icon4py v0.0.2 * GitHub Action: Apply Pep8-formatting * cleanup --------- Co-authored-by: github-actions <[email protected]> * Reverting to clean versions of icon, nvidia blas and lapack * pulling changes from icon-gt4py-daint2 to support latest gt4py * addressing review comments * GitHub Action: Apply Pep8-formatting * bumpin icon4py to 0.0.3, gt4py tag uknown yet. Gt4py requirements now match the correct min requirements * updating tag to temporary tag --------- Co-authored-by: Jonas Jucker <[email protected]> Co-authored-by: github-actions <[email protected]> Co-authored-by: juckerj <[email protected]> Co-authored-by: Samuel Kellerhals <[email protected]> Co-authored-by: Christoph Müller <[email protected]>
abishekg7
added a commit
that referenced
this pull request
Apr 26, 2023
* add versions for icon4py and gt4py * GitHub Action: Apply Pep8-formatting * add fix for py-poetry-core * GitHub Action: Apply Pep8-formatting * updates to get icon4py building after the major gt4py overhaul * Update system_test.py * GitHub Action: Apply Pep8-formatting * add missing test deps, only test next and eve * remove obsolete patch * addressing some review comments * Add Boost to CMake Include Path * Change icon, gridtools and balfrin site packages for successful build * GitHub Action: Apply Pep8-formatting * set boost-path for unittests of icon4py, make tests on Balfrin work * lock python and gt4py versions * GitHub Action: Apply Pep8-formatting * fix typo * GitHub Action: Apply Pep8-formatting * Edits to support icon-dsl on Daint (#693) * working on daint * GitHub Action: Apply Pep8-formatting * tested for icon4py v0.0.2 * GitHub Action: Apply Pep8-formatting * cleanup --------- Co-authored-by: github-actions <[email protected]> * Reverting to clean versions of icon, nvidia blas and lapack * pulling changes from icon-gt4py-daint2 to support latest gt4py * addressing review comments * GitHub Action: Apply Pep8-formatting * icon with dsl variants * GitHub Action: Apply Pep8-formatting * bumpin icon4py to 0.0.3, gt4py tag uknown yet. Gt4py requirements now match the correct min requirements * numpy seems to depend on blas/lapack without this * remove artefact * addressing some review comments * GitHub Action: Apply Pep8-formatting * Update exclaim branch * trying new paths for icon4py * switching all gridtools references to py-gridtools-cpp and bumping to v2.3.0 * GitHub Action: Apply Pep8-formatting * Changing to min requirements for py-gridtools-cpp * fix for cases when dsl=none * GitHub Action: Apply Pep8-formatting * Update repos/c2sm/packages/icon/package.py Co-authored-by: juckerj <[email protected]> --------- Co-authored-by: Jonas Jucker <[email protected]> Co-authored-by: github-actions <[email protected]> Co-authored-by: juckerj <[email protected]> Co-authored-by: Samuel Kellerhals <[email protected]> Co-authored-by: Christoph Müller <[email protected]>
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.
No description provided.