Skip to content

Conversation

@abagusetty
Copy link
Contributor

@abagusetty abagusetty commented Oct 16, 2024

  1. Update Cmake machine config for Aurora
  2. Cleanup certain warnings related to headers and Intel compilers
  3. Linking Fortran/C++/SYCL with CXX linker (-fortlib) instead of Fortran language for SYCL builds. Since Fortran linking with SYCL is no longer supported (https://www.intel.com/content/www/us/en/docs/dpcpp-cpp-compiler/developer-guide-reference/2024-2/fortlib.html)


#ifdef KOKKOS_ENABLE_SYCL
#include <CL/sycl.hpp>
#include <sycl/sycl.hpp>
Copy link
Contributor

Choose a reason for hiding this comment

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

could you explain this change please?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

SYCL specification has long deprecated these headers. <CL/sycl.hpp> header was valid only in SYCL 1.2.1 but it has been deprecated for <sycl/sycl.hpp> in SYCL 2020 Standards.

From the SYCL spec

For consistency, the programming API will only refer to the <sycl/sycl.hpp> header and the ::sycl namespace, but this should be considered synonymous with the SYCL 1.2.1 header and namespace.```


set (USE_NUM_PROCS 12 CACHE STRING "")

SET (USE_MPI_OPTIONS "--pmi=pmix --cpu-bind list:0-7,104-111:8-15,112-119:16-23,120-127:24-31,128-135:32-39,136-143:40-47,144-151:52-59,156-163:60-67,164-171:68-75,172-179:76-83,180-187:84-91,188-195:92-99,196-203 gpu_tile_compact.sh" CACHE FILEPATH "")
Copy link
Contributor

Choose a reason for hiding this comment

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

is tile script always in a user's path?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, the tile binding script is always in the SYSTEM path for the users

@rljacob
Copy link
Member

rljacob commented Oct 16, 2024

To clarify, the changes in components/homme/cmake only impact the standalone build of HOMMEXX but the other changes could impact the EAMxx/SCREAM build?

@abagusetty
Copy link
Contributor Author

To clarify, the changes in components/homme/cmake only impact the standalone build of HOMMEXX but the other changes could impact the EAMxx/SCREAM build?

I guess @oksanaguba could fill-in more accurately, but changes outside the standalone cmake config to HOMME were to address deprecation warnings and so are cosmetic

#define HOMMEXX_VECTOR_PRAGMAS_HPP

#if defined(__INTEL_COMPILER)
#if defined(__INTEL_COMPILER) || defined(__INTEL_CLANG_COMPILER) || defined(__INTEL_LLVM_COMPILER)
Copy link
Contributor

Choose a reason for hiding this comment

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

which one will be INTEL_CLANG compiler and which one is LLVM? thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Intel compilers are identified as both clang and llvm based. Both the macros are defined by oneAPI compilers.

The difference being the naming convention: If it is a compiler released by Intel as a product (both the macros are defined) or if a user builds the compiler themselves from upstream repo (then either or both could be defined)

@oksanaguba
Copy link
Contributor

yes, the changes besides cmake will be picked up by scream builds, but as Abhishek says they are an improvement. SCREAM flags may need revision in a similar way (or not).

@abagusetty
Copy link
Contributor Author

@oksanaguba The CI seems to have failed. Is that what is blocking at the moment

@oksanaguba
Copy link
Contributor

@abagusetty we use a manual process to merge to "next" first and let cdash tests pass overnight https://my.cdash.org/index.php?project=E3SM . then the PR can go to "master". i will merge this to next maybe today. i am not sure about CI. thanks.

@rljacob
Copy link
Member

rljacob commented Oct 23, 2024

@oksanaguba the CI we run on PRs through github actions had a fail which was not related to the PR. I reran it and its fine.

@oksanaguba
Copy link
Contributor

On chrysalis, homme build fails with
5) intel-mkl/2020.4.304-g2qaxzf
4) intel/20.0.4-kodw73g
because of lapack calls undefined reference to dgemm_'` etc. @abagusetty would you want to revert mkl flag, and i will retest?

@abagusetty
Copy link
Contributor Author

On chrysalis, homme build fails with 5) intel-mkl/2020.4.304-g2qaxzf 4) intel/20.0.4-kodw73g because of lapack calls undefined reference to dgemm_'` etc. @abagusetty would you want to revert mkl flag, and i will retest?

Addressed in the following commit

@oksanaguba
Copy link
Contributor

ran homme suites and one ERS test on chrysalis, all ok.

oksanaguba added a commit that referenced this pull request Oct 28, 2024
Update Cmake machine config for Aurora
Cleanup certain warnings related to headers and Intel compilers
Linking Fortran/C++/SYCL with CXX linker (-fortlib) instead of Fortran
language for SYCL builds. Since Fortran linking with SYCL is no longer
supported (link is in the PR).
@oksanaguba oksanaguba merged commit d8a408c into E3SM-Project:master Oct 29, 2024
9 checks passed
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