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

ci: disable power9, add a100 and roci jobs #348

Merged
merged 14 commits into from
Mar 11, 2024
Merged

Conversation

rbberger
Copy link
Collaborator

PR Summary

Closes #347

PR Checklist

  • Adds a test for any bugs fixed. Adds tests for new features.
  • Format your changes by using the make format command after configuring with cmake.
  • Document any new features, update documentation for changes made.
  • Make sure the copyright notice on any files you modified is up to date.
  • After creating a pull request, note it in the CHANGELOG.md file.
  • LANL employees: make sure tests pass both on the github CI and on the Darwin CI

If preparing for a new release, in addition please check the following:

  • Update the version in cmake.
  • Move the changes in the CHANGELOG.md file under a new header for the new release, and reset the categories.

@rbberger rbberger force-pushed the rberger_ci_upgrade branch 5 times, most recently from e9d0569 to 54b615a Compare February 22, 2024 07:38
Copy link
Collaborator

@Yurlungur Yurlungur left a comment

Choose a reason for hiding this comment

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

LGTM let me know when it's ready to merge

@rbberger rbberger force-pushed the rberger_ci_upgrade branch 2 times, most recently from db64436 to b38205c Compare February 28, 2024 15:48
@rbberger rbberger changed the title ci: disable power9 and add roci jobs ci: disable power9 and add a100 and roci jobs Feb 28, 2024
@rbberger rbberger changed the title ci: disable power9 and add a100 and roci jobs ci: disable power9, add a100 and roci jobs Feb 28, 2024
@rbberger rbberger force-pushed the rberger_ci_upgrade branch from cf971c8 to 8070c42 Compare March 6, 2024 20:57
@rbberger rbberger force-pushed the rberger_ci_upgrade branch 2 times, most recently from e84508a to e2999aa Compare March 7, 2024 18:56
@rbberger rbberger force-pushed the rberger_ci_upgrade branch from 1abb487 to c068623 Compare March 8, 2024 17:24
@rbberger
Copy link
Collaborator Author

rbberger commented Mar 8, 2024

@Yurlungur this is now ready. The CI on one of the Rocinante jobs still fails due to some HDF5 issue, but I think this is beyond the scope of this MR which already packs a lot of changes.

@rbberger rbberger marked this pull request as ready for review March 8, 2024 17:26
Copy link
Collaborator

@Yurlungur Yurlungur left a comment

Choose a reason for hiding this comment

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

I'm approving now. However, I'd like to give @ktsai7 @dholladay00 and @mauneyc-LANL to review if they want to. Please review by early next week or forever hold your peace, then I will merge.

@Yurlungur Yurlungur requested a review from ktsai7 March 8, 2024 17:31
@rbberger rbberger force-pushed the rberger_ci_upgrade branch from c068623 to f75632b Compare March 8, 2024 17:44
@rbberger
Copy link
Collaborator Author

rbberger commented Mar 8, 2024

@ktsai7 in particular should look at the different jobs that are still rebuilding various spack packages.

@ktsai7
Copy link
Contributor

ktsai7 commented Mar 11, 2024

@rbberger I will take a look at the packages. @Yurlungur If this is working, I would say to get it in. I am making rounds adding some CI stuff, but that won't conflict with this.

@@ -59,6 +59,8 @@ register_headers(

if (SINGULARITY_BUILD_CLOSURE)
register_headers(closure/mixed_cell_models.hpp)
register_headers(eos/singularity_eos.hpp)
register_srcs(eos/singularity_eos.cpp)
Copy link
Collaborator

Choose a reason for hiding this comment

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

what necessitated this change?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

I didn't change this, @rbberger did, but I think he just moved these registration calls from the fortran if statement to the closure if statement. Since this is C++ code technically, not fortran code.

Copy link
Collaborator

Choose a reason for hiding this comment

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

this will blow up compile times in RIOT. While it is c++ source, they are really only needed in w/in the context of defining fortran interfaces and for EAP.

Copy link
Collaborator

Choose a reason for hiding this comment

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

oh this is a good catch. Thank you! Yes let's move it back into the fortran. I can submit a PR. @rbberger will this be ok?

@Yurlungur Yurlungur merged commit 3684eae into main Mar 11, 2024
4 checks passed
@Yurlungur Yurlungur deleted the rberger_ci_upgrade branch March 11, 2024 18:34
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.

Remove power9 runners. Add Roci runners.
4 participants