-
Notifications
You must be signed in to change notification settings - Fork 19
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
Working gt4py + icon4py on Balfrin and Daint #715
Conversation
* 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]>
|
@@ -15,40 +15,46 @@ class PyGt4py(PythonPackage): | |||
|
|||
url = "ssh://[email protected]/GridTools/gt4py.git" | |||
|
|||
version('functional', branch='functional', git=url) | |||
version('main', branch='main', git=url) | |||
version('1.0.1', tag='v1.0.1', git=url) |
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.
This will be the release we test in spack-c2sm?
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.
So the TL;DR of this is, our icon4py and gt4py developments are currently in flux, and we won't have usable tags for a week or two. I'm just specifying the main branches for both here so that we have at least something working for now. This will not be the version that goes out to the users.
|
||
homepage = "https://github.com/C2SM/icon4py" | ||
|
||
maintainers = ['samkellerhals'] | ||
|
||
version('main', branch='main', git='ssh://[email protected]/C2SM/icon4py.git') | ||
version('main', branch='main', git=git) | ||
version('0.0.1', tag='v0.0.1', git=git) |
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.
If we don't use it I suggest to delete it
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.
Deleting it for now. New tags will follow in a week or so.
version('main', branch='main', git='ssh://[email protected]/C2SM/icon4py.git') | ||
version('main', branch='main', git=git) | ||
version('0.0.1', tag='v0.0.1', git=git) | ||
version('0.0.2', tag='v0.0.2', git=git) |
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.
We test this?
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.
Deleting it for now. New tags will follow in a week or so.
@@ -27,8 +30,13 @@ class PyIcon4py(PythonPackage): | |||
depends_on('[email protected]:', type=('build', 'run')) | |||
# TODO: push new version to Spack official | |||
depends_on('[email protected]:', type=('build', 'run')) | |||
depends_on('py-gt4py', type=('build', 'run')) | |||
depends_on('py-gt4py@main:', type=('build', 'run')) |
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.
I don't like that. main:
has not really any meaning because it is not a number.
This also makes the life of the conretizer more difficult.
Either we stick to concrete version like 0.0.1:
or we don't specifiy it at all, and push that responsability to users or an environment file
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.
main:
was a mistake. Removing the colon.
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.
I strongly recommend to hardcode the dependency to a branch that evolves on a daily base.
test/system_test.py
Outdated
@pytest.mark.no_tsa # py-isort install fails with: No module named 'poetry'. | ||
class PyIcon4pyTest(unittest.TestCase): | ||
|
||
def test_install_version_main(self): | ||
spack_install_and_test('py-icon4py @main %gcc') | ||
def test_install_version_0_0_1(self): |
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.
Same here
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.
changed both tests to main for now. they seem to pass on Daint at least.
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.
We don't want tests of branches in spack-c2sm.
That was the reason why we insisted on using tags.
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.
Please choose the versions used for icon4py/gt4py wisely. This will prevent us from a big mess.
Otherwise clean-up a bit and make sure the tests are meaningful.
launch jenkins balfrin daint py-icon4py py-gt4py |
tsa🟢 unit test
|
balfrin🟢 unit test
🟢 integration test
🟢 system test
|
daint🟢 unit test
🟢 integration test
🟢 system test
|
@abishekg7 Cool the test pass on both machines. |
To check if branch main is working I propose the following: |
… match the correct min requirements
gt4py is merged, but I'm waiting on the tag. I've just put in a placeholder tag here that needs updating. But the gt4py main branch seems to build along with icon4py v0.0.3 |
launch jenkins balfrin daint py-icon4py py-gt4py |
tsa🟢 unit test
|
balfrin🟢 unit test
🟢 integration test
🟢 system test
|
daint🟢 unit test
🟢 integration test
🟢 system test
|
No description provided.