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

EAM: fix cmake configuration of crm/pam #7008

Merged
merged 3 commits into from
Feb 18, 2025

Conversation

bartgol
Copy link
Contributor

@bartgol bartgol commented Feb 13, 2025

Fixes CI error in building MMF case (see e.g. this build)

[BFB]


Details:

  • pam does not need eamxx_physics, since it already links against p3 and shoc
  • the submod pam had a problem with correctly getting SCREAM_DATA_DIR into the scream_config.h file

@bartgol bartgol added BFB PR leaves answers BFB EAMxx PRs focused on capabilities for EAMxx labels Feb 13, 2025
@bartgol bartgol requested a review from mahf708 February 13, 2025 15:30
@bartgol bartgol self-assigned this Feb 13, 2025
@bartgol bartgol requested a review from jgfouca February 13, 2025 19:05
@bartgol
Copy link
Contributor Author

bartgol commented Feb 13, 2025

This is still not fixing the MMF2 fail. It appears that SCREAM_DATA_DIR is undefined by the time cmake generates scream_config.h, causing a runtime error b/c the p3 table filename is ill-formed. I'm digging.

@bartgol
Copy link
Contributor Author

bartgol commented Feb 14, 2025

Turns out I needed another PR in PAM. When that goes in, I'll update this branch. Also, we don't need to rm the eamxx_physics target in eamxx, so I'll rewrite the git history when I update PAM as well.

@bartgol bartgol marked this pull request as draft February 14, 2025 01:25
@bartgol bartgol force-pushed the bartgol/eamxx/rm-eamxx-physics-lib branch from f7ca589 to 7f09c9a Compare February 14, 2025 17:00
@bartgol bartgol changed the title EAMxx: remove interface library eamxx_physics EAM: fix cmake configuration of crm/pam Feb 14, 2025
@bartgol bartgol requested a review from jgfouca February 14, 2025 17:05
bartgol added a commit that referenced this pull request Feb 14, 2025
@bartgol bartgol marked this pull request as ready for review February 14, 2025 20:27
@bartgol
Copy link
Contributor Author

bartgol commented Feb 17, 2025

The fix did not work. I just pushed another commit, and am trying to build on mappy to ensure this time we got it...

Edit: on mappy, the case now builds. It's stuck in the q for the run phase, but at least it now builds.

Edit 2: the test passes on mappy. I cleaned up the PR and re-merged to next.

Gets some CMake fixes when building with EAMxx physics
* EAMxx cmake already declares one
* PAM does not need to link against eamxx_physics,
  as it already links against p3 and shoc
PAM does not include src/physics, so the tgt is never declared
@bartgol bartgol force-pushed the bartgol/eamxx/rm-eamxx-physics-lib branch from a4ca426 to ae9120b Compare February 17, 2025 18:54
@bartgol bartgol requested a review from jgfouca February 18, 2025 17:41
@bartgol
Copy link
Contributor Author

bartgol commented Feb 18, 2025

Since manually running the MMF2 test passes on mappy, I am going to merge this. SNL jenkins was sick yesterday, and the MMF2 doesn't even build on master now, so merging this can only make things better anyways.

@bartgol bartgol merged commit 5bb943c into master Feb 18, 2025
15 of 19 checks passed
@bartgol bartgol deleted the bartgol/eamxx/rm-eamxx-physics-lib branch February 18, 2025 18:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BFB PR leaves answers BFB EAMxx PRs focused on capabilities for EAMxx
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants