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

Remove REQUIRED keyword from find_package(MPI) #52

Merged
merged 1 commit into from
Jul 6, 2021

Conversation

MuellerSeb
Copy link
Contributor

@MuellerSeb MuellerSeb commented Jun 17, 2021

Closes #50

Using this with the described setting of #50, I get the following messages:

-- Could NOT find MPI_Fortran (missing: MPI_Fortran_WORKS) 
-- Could NOT find MPI (missing: MPI_Fortran_FOUND) 
    Reason given by package: MPI component 'C' was requested, but language C is not enabled.  MPI component 'CXX' was requested, but language CXX is not enabled.  

but compilation works afterwards.

@MuellerSeb
Copy link
Contributor Author

I think merging this is fine, since MPI is later on only linked if the desired component is found.

@@ -183,7 +183,7 @@ else()
endif()

if(NetCDF_PARALLEL)
find_package(MPI REQUIRED)
find_package(MPI)
Copy link
Collaborator

Choose a reason for hiding this comment

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

If NetCDF_PARALLEL is True and MPI is not found, wouldn't the linkages of the netcdf library be incomplete?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the case described in #50, this works.
On the other hand, what is the reasoning for the second check for the MPI components then?

if(MPI_${_comp}_FOUND)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kgerheiser also said, this would be fine: #50 (comment)

@kgerheiser kgerheiser self-requested a review July 6, 2021 14:06
@kgerheiser
Copy link
Contributor

Will merge then

@kgerheiser kgerheiser merged commit d39f199 into NOAA-EMC:develop Jul 6, 2021
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.

Why forcing parallel usage
3 participants