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

Phoenix Runner #41

Merged
merged 54 commits into from
Jun 25, 2024
Merged

Phoenix Runner #41

merged 54 commits into from
Jun 25, 2024

Conversation

suzanmanasreh
Copy link
Collaborator

@suzanmanasreh suzanmanasreh commented Jun 19, 2024

All the updates in this PR:

  • Top-level CMakeLists.txt
    • /common CMakeLists.txt because I added common as a subdirectory (easy re-factor, but I thought it made sense for a library)
    • cases are custom targets with their own binary directory inside /build
    • warnings if it can't find packages
    • support both module loaded PETSc and package installed PETSc (I don't think the PETSC_INCLUDE_DIRS naturally handles both, so this is messy sorry)
  • Case file renames
  • Kept old Makefiles to leave that option
  • Readme updates
    • This is confusing since the install script that works on my account doesn't work on all Phoenix accounts.
  • Working Phoenix runner
    • Had to custom install Lapack and FFTW in /packages for ml gcc/12.1.0-qgxpzk mvapich2/2.3.7-733lcv to work since the FFTW module depends on mvapich2/2.3.6-ouywal, and that one is broken inside the Phoenix runner. This install script is now under install-phoenix.sh, but it’s more of a worst-case scenario because install.sh works for me on Phoenix. install.sh does work on the ICE runner because the ICE FFTW depends on the default mvapich2/2.3.7-1 already.
    • Tests both with Makefiles and CMake. Makefile.in isn't flexible at all though (there's nothing I can do about this), so it doesn't work with all builds.

@sbryngelson
Copy link
Member

please rebase so there aren't so many commits

@suzanmanasreh
Copy link
Collaborator Author

i took out the the line that lets you see a PR's commits on master but okay i'll try. the commit history on this is really complicated from having to add and remove my own cases.

@sbryngelson
Copy link
Member

this has commits from 3 weeks ago?

@suzanmanasreh
Copy link
Collaborator Author

suzanmanasreh commented Jun 19, 2024

yeah i basically kept the same branch that i made other PRs from but ran a "git merge master" after pulling the PR onto master and never started a new branch, so the real differences start after #8ec18f5. sorry this is so hacky, i'm trying to resolve conflicts in the rebase now, but there's conflicts in all 96 commits, so it's too cumbersome.

@suzanmanasreh
Copy link
Collaborator Author

suzanmanasreh commented Jun 20, 2024

I just reset the commit history. Some of the Makefiles on the branch might look old, but that was because I gave all the case files unique names, so a global Makefile wasn't gonna work anymore without some wildcards. I'm hoping Makefiles can be completely removed in the future though if I'm happy with cmake. It handles everything pretty dynamically, but if you need a better explanation of the system I set up, I can try!

@suzanmanasreh
Copy link
Collaborator Author

suzanmanasreh commented Jun 20, 2024

Maybe someone else could help with the review if they're familiar with cmake? It's pretty much identical to Makefile.in and PETSc's official cmake.

@suzanmanasreh suzanmanasreh linked an issue Jun 21, 2024 that may be closed by this pull request
4 tasks
@suzanmanasreh suzanmanasreh changed the title Experimental CMake CMake Addition Jun 21, 2024
@suzanmanasreh suzanmanasreh marked this pull request as draft June 21, 2024 19:03
@suzanmanasreh suzanmanasreh marked this pull request as ready for review June 22, 2024 01:31
@suzanmanasreh
Copy link
Collaborator Author

suzanmanasreh commented Jun 22, 2024

@sbryngelson look Phoenix runner works! I'm not putting a MacOS runner in the same PR though cause there's already too much here. The CMake works on Mac too though.

@suzanmanasreh suzanmanasreh marked this pull request as ready for review June 22, 2024 17:01
@suzanmanasreh suzanmanasreh changed the title CMake Addition Phoenix Runner Jun 24, 2024
@suzanmanasreh suzanmanasreh requested review from sbryngelson and removed request for sbryngelson June 25, 2024 02:09
@sbryngelson sbryngelson merged commit 4071322 into master Jun 25, 2024
6 checks passed
@sbryngelson sbryngelson deleted the newbranch branch June 25, 2024 02:14
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.

Dependencies are outdated
2 participants