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

add MPI include directories and definitions #962

Closed
wants to merge 1 commit into from

Conversation

jmlapre
Copy link
Member

@jmlapre jmlapre commented Jun 22, 2023

Experimental CMake config omits some important commands:

  include_directories(SYSTEM ${MPI_INCLUDE_PATH})
  add_definitions(-DSST_CONFIG_HAVE_MPI)

@github-actions github-actions bot added AT: WIP Mark PR as a Work in Progress (No Autotesting Performed) AT: CLANG-FORMAT PASS and removed AT: WIP Mark PR as a Work in Progress (No Autotesting Performed) labels Jun 22, 2023
@github-actions
Copy link

CLANG-FORMAT TEST - PASSED

@jpkenny jpkenny self-requested a review June 22, 2023 16:27
Copy link
Contributor

@jpkenny jpkenny left a comment

Choose a reason for hiding this comment

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

If one uses the MPI compiler wrappers, which I believe is standard practice these days, adding the include directories shouldn't be necessary. What situation is the "include_directories" fixing?

@berquist
Copy link
Member

The way to avoid the SYSTEM include is to link SST::SST or whatever the target is called to MPI::MPI via target_link_libraries. If that doesn't work, then something is wrong the SST target.

@jmlapre
Copy link
Member Author

jmlapre commented Jun 22, 2023

If one uses the MPI compiler wrappers, which I believe is standard practice these days, adding the include directories shouldn't be necessary. What situation is the "include_directories" fixing?

When following the instructions in the README it mentions running cmake like so:

cmake ../experimental

which will not work. Also, setting CXX to mpicxx should be avoided as per this comment from the maintainer of the FindMPI cmake module.

@jpkenny
Copy link
Contributor

jpkenny commented Jun 22, 2023

@jmlapre And isn't SST_CONFIG_HAVE_MPI already defined in sst_config.h?

@jpkenny
Copy link
Contributor

jpkenny commented Jun 22, 2023

@jmlapre

Also, setting CXX to mpicxx should be avoided as per this comment from the maintainer of the FindMPI cmake module.

Ok, following the "general advice" from that comment is going to be very error prone because most users don't even know what the "compiler you want to work with" is. That said, if they just set CXX to mpicxx they're normally going to be fine for common mpi implementations.

However, I don't think this changeset is a sufficient fix for mpi implementations that don't provide wrappers -- at a minimum I assume MPI_CXX_LIBRARIES etc need to be added to whatever the LD_FLAGS equivalent is.

@sst-autotester
Copy link
Contributor

Status Flag 'Pull Request AutoTester' - Testing Jenkins Projects:

Pull Request Auto Testing STARTING (click to expand)

Build Information

Test Name: SST__AutotestGen2_NewFW_sst-test_OMPI-4.1.4_PY3.6_sst-elements

  • Build Num: 1056
  • Status: STARTED

Build Information

Test Name: SST__AutotestGen2_NewFW_sst-test_OMPI-4.1.4_PY3.6_sst-elements_MR-2

  • Build Num: 1040
  • Status: STARTED

Build Information

Test Name: SST__AutotestGen2_NewFW_sst-test_OMPI-4.1.4_PY3.6_sst-elements_MT-2

  • Build Num: 1042
  • Status: STARTED

Build Information

Test Name: SST__AutotestGen2_NewFW_sst-test_OMPI-4.1.4_PY3.6_sst-macro_withsstcore

  • Build Num: 432
  • Status: STARTED

Build Information

Test Name: SST__AutotestGen2_NewFW_sst-test_OMPI-4.1.4_PY3.6_sst-core

  • Build Num: 354
  • Status: STARTED

Build Information

Test Name: SST__AutotestGen2_NewFW_sst-test_OMPI-4.1.4_PY3.6_sst-core_Make-Dist

  • Build Num: 312
  • Status: STARTED

Build Information

Test Name: SST__AutotestGen2_NewFW_sst-test_Clang-Format_sst-core

  • Build Num: 266
  • Status: STARTED

Build Information

Test Name: SST__AutotestGen2_NewFW_OSX-13.2-XC14-ARM_OMPI-4.1.4_PY3.10_sst-elements

  • Build Num: 396
  • Status: STARTED

Build Information

Test Name: SST__AutotestGen2_NewFW_OSX-13.2-XC14-ARM_OMPI-4.1.4_PY3.10_sst-macro_withsstcore

  • Build Num: 99
  • Status: STARTED

Using Repos:

Repo: CORE (jmlapre/sst-core)
  • Branch: add-mpi-include-dirs
  • SHA: 7dd51c9
  • Mode: TEST_REPO
Repo: SQE (sstsimulator/sst-sqe)
  • Branch: devel
  • SHA: 85e037a1050c1dd8d3c56eabb231daa41c5be27d
  • Mode: SUPPORT_REPO
Repo: ELEMENTS (sstsimulator/sst-elements)
  • Branch: devel
  • SHA: 90aeae490ab01ae9582b8adcbd4a5fecef4b48b8
  • Mode: SUPPORT_REPO
Repo: MACRO (sstsimulator/sst-macro)
  • Branch: devel
  • SHA: 432426e6e6a709368feb9abc6f91c654ffad9b6f
  • Mode: SUPPORT_REPO

Pull Request Author: jmlapre

@sst-autotester
Copy link
Contributor

Status Flag 'Pull Request AutoTester' - Jenkins Testing: 1 or more Jobs FAILED

Note: Testing will normally be attempted again in approx. 4 Hrs. If a change to the PR source branch occurs, the testing will be attempted again on next available autotester run.

Pull Request Auto Testing has FAILED (click to expand)

Job: SST__AutotestGen2_NewFW_sst-test_OMPI-4.1.4_PY3.6_sst-elements

  • Result: PASSED
  • Build #: 1056
  • URL: Jenkins server at https://sst-jenkins.sandia.gov/view/SST/job/SST__AutotestGen2_NewFW_sst-test_OMPI-4.1.4_PY3.6_sst-elements/1056/consoleFull

Job: SST__AutotestGen2_NewFW_sst-test_OMPI-4.1.4_PY3.6_sst-elements_MR-2

  • Result: PASSED
  • Build #: 1040
  • URL: Jenkins server at https://sst-jenkins.sandia.gov/view/SST/job/SST__AutotestGen2_NewFW_sst-test_OMPI-4.1.4_PY3.6_sst-elements_MR-2/1040/consoleFull

Job: SST__AutotestGen2_NewFW_sst-test_OMPI-4.1.4_PY3.6_sst-elements_MT-2

  • Result: PASSED
  • Build #: 1042
  • URL: Jenkins server at https://sst-jenkins.sandia.gov/view/SST/job/SST__AutotestGen2_NewFW_sst-test_OMPI-4.1.4_PY3.6_sst-elements_MT-2/1042/consoleFull

Job: SST__AutotestGen2_NewFW_sst-test_OMPI-4.1.4_PY3.6_sst-macro_withsstcore

  • Result: PASSED
  • Build #: 432
  • URL: Jenkins server at https://sst-jenkins.sandia.gov/view/SST/job/SST__AutotestGen2_NewFW_sst-test_OMPI-4.1.4_PY3.6_sst-macro_withsstcore/432/consoleFull

Job: SST__AutotestGen2_NewFW_sst-test_OMPI-4.1.4_PY3.6_sst-core

  • Result: PASSED
  • Build #: 354
  • URL: Jenkins server at https://sst-jenkins.sandia.gov/view/SST/job/SST__AutotestGen2_NewFW_sst-test_OMPI-4.1.4_PY3.6_sst-core/354/consoleFull

Job: SST__AutotestGen2_NewFW_sst-test_OMPI-4.1.4_PY3.6_sst-core_Make-Dist

  • Result: PASSED
  • Build #: 312
  • URL: Jenkins server at https://sst-jenkins.sandia.gov/view/SST/job/SST__AutotestGen2_NewFW_sst-test_OMPI-4.1.4_PY3.6_sst-core_Make-Dist/312/consoleFull

Job: SST__AutotestGen2_NewFW_sst-test_Clang-Format_sst-core

  • Result: PASSED
  • Build #: 266
  • URL: Jenkins server at https://sst-jenkins.sandia.gov/view/SST/job/SST__AutotestGen2_NewFW_sst-test_Clang-Format_sst-core/266/consoleFull

Job: SST__AutotestGen2_NewFW_OSX-13.2-XC14-ARM_OMPI-4.1.4_PY3.10_sst-elements

  • Result: FAILED
  • Build #: 396
  • URL: Jenkins server at https://sst-jenkins.sandia.gov/view/SST/job/SST__AutotestGen2_NewFW_OSX-13.2-XC14-ARM_OMPI-4.1.4_PY3.10_sst-elements/396/consoleFull
  • Job: - Status: FAILURE

Job: SST__AutotestGen2_NewFW_OSX-13.2-XC14-ARM_OMPI-4.1.4_PY3.10_sst-macro_withsstcore

  • Result: FAILED
  • Build #: 99
  • URL: Jenkins server at https://sst-jenkins.sandia.gov/view/SST/job/SST__AutotestGen2_NewFW_OSX-13.2-XC14-ARM_OMPI-4.1.4_PY3.10_sst-macro_withsstcore/99/consoleFull
  • Job: - Status: FAILURE

@jpkenny jpkenny added the AT: WIP Mark PR as a Work in Progress (No Autotesting Performed) label Jun 23, 2023
@jmlapre jmlapre closed this Oct 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AT: CLANG-FORMAT PASS AT: WIP Mark PR as a Work in Progress (No Autotesting Performed)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants