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

miscellaneous fixes #71

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gcongiu
Copy link

@gcongiu gcongiu commented Aug 22, 2023

  • use pthread instread of omp in multi_monitor.cpp test
  • fix path for hipcc
  • use papi@master as default version of papi
  • print the test name in run script

@eugeneswalker
Copy link
Collaborator

Thank you for this!

We are deploying E4S 23.08 on a number of systems right now. I will try this updated test and report back here ASAP.

@gcongiu
Copy link
Author

gcongiu commented Sep 26, 2023

@eugeneswalker I added a couple more fixes as I notices the run.sh script was not setting up the environment for the test correctly, as result the tests were not picking up the spack PAPI but the system installed PAPI.

- use pthread instread of omp in multi_monitor.cpp test
- fix path for hipcc
- use papi@master as default version of papi
- print the test name in run script
@eugeneswalker
Copy link
Collaborator

@eugeneswalker I added a couple more fixes as I notices the run.sh script was not setting up the environment for the test correctly, as result the tests were not picking up the spack PAPI but the system installed PAPI.

One problem I just noticed is, we do not as a policy deploy branch versions of packages as part of E4S, so papi@master would not be available in our facility deployments. We only deploy tagged or versioned releases.

@gcongiu
Copy link
Author

gcongiu commented Sep 27, 2023

@eugeneswalker I added a couple more fixes as I notices the run.sh script was not setting up the environment for the test correctly, as result the tests were not picking up the spack PAPI but the system installed PAPI.

One problem I just noticed is, we do not as a policy deploy branch versions of packages as part of E4S, so papi@master would not be available in our facility deployments. We only deploy tagged or versioned releases.

Ok makes sense. We can “suspend” this PR until next Papi release.

@eugeneswalker eugeneswalker marked this pull request as draft September 27, 2023 19:42
@eugeneswalker
Copy link
Collaborator

Ok makes sense. We can “suspend” this PR until next Papi release.

Sounds good. I just converted it to a draft for now. Let us revive ASAP.

@wspear
Copy link
Collaborator

wspear commented May 15, 2024

I see that we are releasing e4s with papi@7.1.0 now. Are we good to finalize and commit this change (maybe specifying papi@7.0.0: ?)

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.

3 participants