Skip to content
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

Use ip@5: if available instead of sp #524

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

climbfuji
Copy link
Collaborator

The sp library is deprecated and replaced by ip@5 and newer (drop-in replacement). While [email protected] is still in spack-stack-1.8.0, ip-5.0.0 is also available. We expect sp to be removed in spack-stack-1.9.0.

This PR goes with NCAR/ccpp-physics#1090

Copy link
Collaborator

@dustinswales dustinswales left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All tests pass.
Looks good to me.

@climbfuji
Copy link
Collaborator Author

ping

@grantfirl
Copy link
Collaborator

@scrasmussen I saw #551. I'm going to run some tests on Hera to verify that this works with spack-stack 1.8.0 before I approve. If it works, we can merge it, although I don't think that we want the SCM to get ahead of where the UFS is in terms of which version spack-stack is being used by default: https://github.com/ufs-community/ufs-weather-model/blob/develop/modulefiles/ufs_hera.intel.lua

@scrasmussen
Copy link
Member

@grantfirl If we don't want the SCM to get ahead I can just answer gabersek with instructions to checkout my updated fature/ip5 branch and use that as a workaround until the default spack-stack gets updated and we merge this? (Hopefully I'm understanding the situation correctly)

@grantfirl
Copy link
Collaborator

@grantfirl If we don't want the SCM to get ahead I can just answer gabersek with instructions to checkout my updated fature/ip5 branch and use that as a workaround until the default spack-stack gets updated and we merge this? (Hopefully I'm understanding the situation correctly)

I think that it should be OK to merge before UFS updates their spack-stack version because Dom changed the CMakeLists to look for IP first and then SP if not found. This should work fine with spack-stack 1.6 until it is updated to coincide with UFS. gabersek was already doing something "nonstandard" by trying the SCM with spack-stack 1.8, but it wouldn't work because CMakeLists.txt didn't even know to look for IP yet. It's weird, though, because sp supposedly still exists in 1.8, so I'm not 100% sure why the issue cropped up yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants