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

Rework the dependency linking of hydro CMake compilation #2178

Open
wants to merge 1 commit into
base: release-v4.7.0
Choose a base branch
from

Conversation

islas
Copy link
Collaborator

@islas islas commented Mar 7, 2025

TYPE: bug fix

KEYWORDS: cmake, hydro

SOURCE: internal

DESCRIPTION OF CHANGES:
Problem:
Some dependencies were not being fully satisfied and were implicit / partially fulfilled by subsequent compilation. In compilers that could tolerate these situations the hydro folder would compile successfully.

However, for compilers where dependencies are enforced (i.e. symbols resloved at compile time even within Fortran submodules), compilation would fail.

Solution:
To account for situations where compilers require all module dependencies be resolved, dependencies between the hydro libraries and the transitive link properties were reworked to propagate dependencies up the chain correctly.

TESTS CONDUCTED:

  1. Tested building hydro with Intel oneAPI ifx 2024.2.1

RELEASE NOTE:
Fixed failed compilation with Intel oneAPI by reworking the dependency linking of hydro CMake compilation

Some dependencies were not being fully satisfied and were implicit /
partially fulfilled by subsequent compilation. In compilers that could
tolerate these situations the hydro folder would compile successfully.

However, for compilers where dependencies are enforced (i.e. symbols
resloved at compile time even within Fortran submodules), compilation
would fail. To account for these situations, dependencies between the
hydro libraries and the transitive link properties were reworked to
propagate dependencies up the chain correctly.
@islas islas requested a review from a team as a code owner March 7, 2025 18:11
@scrasmussen
Copy link
Contributor

@islas what modules and commands are you using to configure this? I can get it to build and run with GNU but with Intel I'm getting the following errors during the configure step. I'm using the following command and get the error with -DENABLE_HYDRO=ON and without Hydro:

$ ./configure_new -x -p ifx/icx -- -DWRF_CORE=ARW -DWRF_NESTING=BASIC -DWRF_CASE=EM_REAL
...

-- Performing Check FSEEKO
CMake Warning (dev) at cmake/confcheck.cmake:30 (try_run):
  Unknown arguments:

    "-D_FILE_OFFSET_BITS=64"
    "-D_LARGEFILE_SOURCE=1"
    "-DFILE_TO_TEST="/glade/work/soren/src/wrf-hydro/prs/wrf/WRF-islas/CMakeLists.txt""
Call Stack (most recent call first):
  confcheck/CMakeLists.txt:51 (wrf_conf_check)
This warning is for project developers.  Use -Wno-dev to suppress it.

CMake Error: The source directory "/glade/work/soren/src/wrf-hydro/prs/wrf/WRF-islas/tools/fseek_test.c" is a file, not a directory.
Specify --help for usage, or press the help button on the CMake GUI.
CMake Error at cmake/confcheck.cmake:30 (try_run):
  Failed to configure test project build system.
Call Stack (most recent call first):
  confcheck/CMakeLists.txt:51 (wrf_conf_check)


CMake Error: TRY_COMPILE attempt to remove -rf directory that does not contain CMakeTmp or CMakeScratch: "/glade/work/soren/src/wrf-hydro/prs/wrf/WRF-islas/_build/confcheck/confcheck/FSEEKO/"
-- Configuring incomplete, errors occurred!

Think it might be related to this commit? 5dd2c19

@islas
Copy link
Collaborator Author

islas commented Mar 7, 2025

@scrasmussen Yes, I tested these changes only on top of v4.6.1, so I think that subsequent commit broke things for ifx on Derecho.

The following two PRs are meant to address issues with ifx :
PR #2179
PR #2180

With those independent changes merged onto this, you should be able to test this fix for hydro.

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.

2 participants