-
Notifications
You must be signed in to change notification settings - Fork 24
Paraview recipe #39
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
base: main
Are you sure you want to change the base?
Paraview recipe #39
Conversation
installation on ALPS will require two additional items for run-time enabling of the NVIDIA IndeX libraries and license.
|
The PR can now be considered for merging. We support
|
cscs-ci run alps;system=daint;uarch=gh200;uenv=paraview:5.13.0 |
cscs-ci run alps;system=eiger;uarch=zen2;uenv=paraview:5.13.0 |
1 similar comment
cscs-ci run alps;system=eiger;uarch=zen2;uenv=paraview:5.13.0 |
cscs-ci run alps;system=daint;uarch=gh200;uenv=paraview:5.13.0 |
4 similar comments
cscs-ci run alps;system=daint;uarch=gh200;uenv=paraview:5.13.0 |
cscs-ci run alps;system=daint;uarch=gh200;uenv=paraview:5.13.0 |
cscs-ci run alps;system=daint;uarch=gh200;uenv=paraview:5.13.0 |
cscs-ci run alps;system=daint;uarch=gh200;uenv=paraview:5.13.0 |
cscs-ci run alps;system=eiger;uarch=zen2;uenv=paraview:5.13.0 |
cscs-ci run alps;system=daint;uarch=gh200;uenv=paraview:5.13.0 |
Please test this and consider it for deployment on our clusters |
cscs-ci run alps;system=daint;uarch=gh200;uenv=paraview:5.13.0 |
cscs-ci run alps;system=eiger;uarch=zen2;uenv=paraview:5.13.0 |
cscs-ci run alps;system=eiger;uarch=zen2;uenv=paraview:5.13.1 |
please note, paraview release has been bumped from 5.13.0 to 5.13.1 |
cscs-ci run alps;system=eiger;uarch=zen2;uenv=paraview:5.13.1 |
cscs-ci run alps;system=todi;uarch=gh200;uenv=paraview:5.13.1 |
cscs-ci run alps;system=eiger;uarch=zen2;uenv=paraview:5.13.1 |
cscs-ci run alps;system=todi;uarch=gh200;uenv=paraview:5.13.1 |
I've just tried to build it manually on eiger, there is an error compiling h5hut on zen2 too. |
PR comments addressed (hopefully) in commits pushed just now. |
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.
Thanks for all the hard work in figuring out all the details of this build! 💪
We are not going to polish this to be optimal, we will have a follow up PR in case to refine details (e.g. conduit
spack package changes can be pushed to the public repo) and complete the cleanup.
I know we are targeting to merge a Paraview recipe as soon as possible, and I know it took very long to get here.
Thanks again!
- blaspp | ||
- eigen | ||
- fftw | ||
- lapackpp | ||
- openblas | ||
- proj |
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.
are blaspp
, lapackpp
, openblas
, ... paraview
dependencies? Or are there for another legit reason?
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.
Proj is needed, eigen is needed, the others were snuck in as part of my testing, but they don't do any harm. They can be removed, but IMHO they are harmless and useful for people creating plugins that need them.
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.
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 know what you mean...but CDI is required to build the ICON reader
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 mean it is already in the spack official repo, this seems just a superfluous clone of it. In case it is not, it can stay 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.
The main problem is that this was added before the CDI was available/functional in the general spack repo, if we remove this and switch to the built in one, then we needd to do a round of testing and checking to make sure that the cdi parallel load client/server etc etc works ok. It would be better to commit this stuff "as is" - and when we have reframe testing setup, remove all the obsolete things we added one by one as need/requests arise
@@ -0,0 +1,5 @@ | |||
bootstrap: | |||
spec: gcc@11 |
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.
is there any legit reason why we bootstrap gcc@12
with gcc@11
?
spec: gcc@11 | |
spec: gcc@12 |
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.
it couold be updated, it was there since the original 2023 PR commits, if we change it, we ought to rerun tests etc etc -- can we just leave it alone.
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.
aha - that's the eiger build only - the gh200 build was updated with gcc12, but I vaguely remember that eiger needed 11 - I will update it and trigger builds and see if all works.
@@ -0,0 +1,90 @@ | |||
#!/bin/bash |
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 agree with @msimberg on this.
My 2cents
Does it do any harm to have them in the repo?
We might end up with tens of different scripts (there are already a good amount to support the point), one per system, that no one will maintain and probably use because they would have to study/change/update them first. So, maybe they look unharmful, but they already are harmful to the cleanliness of the recipe.
cscs-ci run alps;system=eiger;uarch=zen2;uenv=paraview:5.13.2 |
cscs-ci run alps;system=daint;uarch=gh200;uenv=paraview:5.13.2 |
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.
Everything considered, I agree with what @biddisco said
It would be better to commit this stuff "as is" - and when we have reframe testing setup, remove all the obsolete things we added one by one as need/requests arise
as soon as reframe tests will be ready, we can eventually fix/cleanup/refine this in a next PR.
cscs-ci run alps;system=eiger;uarch=zen2;uenv=paraview:5.13.2 |
cscs-ci run alps;system=daint;uarch=gh200;uenv=paraview:5.13.2 |
…envs are not stored
I tried adding openvdb, ( c.f. https://jira.cscs.ch/browse/SD-65739 )but it did not work, I have removed that commit and will work on that separately. |
cscs-ci run alps;system=eiger;uarch=zen2;uenv=paraview:5.13.2 |
cscs-ci run alps;system=daint;uarch=gh200;uenv=paraview:5.13.2 |
cscs-ci run alps;system=daint;uarch=gh200;uenv=paraview:5.13.2 |
cscs-ci run alps;system=eiger;uarch=zen2;uenv=paraview:5.13.2 |
cscs-ci run alps;system=daint;uarch=gh200;uenv=paraview:5.13.2 |
cscs-ci run alps;system=eiger;uarch=zen2;uenv=paraview:5.13.2 |
cscs-ci run alps;system=daint;uarch=gh200;uenv=paraview:5.13.2 |
5 similar comments
cscs-ci run alps;system=daint;uarch=gh200;uenv=paraview:5.13.2 |
cscs-ci run alps;system=daint;uarch=gh200;uenv=paraview:5.13.2 |
cscs-ci run alps;system=daint;uarch=gh200;uenv=paraview:5.13.2 |
cscs-ci run alps;system=daint;uarch=gh200;uenv=paraview:5.13.2 |
cscs-ci run alps;system=daint;uarch=gh200;uenv=paraview:5.13.2 |
cscs-ci run alps;system=daint;uarch=gh200;uenv=paraview:5.13.2 |
This PR builds a minimal paraview using EGL on daint and eiger.