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

Making OpenFAST test more portable and allowing for linking against shared libraries #299

Merged
merged 8 commits into from
Jan 18, 2018

Conversation

jrood-nrel
Copy link
Contributor

This pull request addresses #296 and #274 .

For #296 this moves the servo library and other necessary files for the OpenFAST test from the mesh directory into the test directory itself, and more importantly the CMAKE_BINARY_DIR, so that the file output generated from OpenFAST itself does not dirty the mesh directory or the Nalu source directory. This also addresses the portability of this test to allow it to run on OSX since it now builds the servo library during the regular build process for the current machine.

I apologize I did not make this two pull requests since, while I was at it, I also decided to address #274 by removing the hardcoded lib CMAKE_FIND_LIBRARY_PREFIXES and .a CMAKE_FIND_LIBRARY_SUFFIXES. This required me to put the TPL package finding after the call to project(Nalu) so that the system specific prefixes and suffixes would already be set. I DID NOT do this for Trilinos however (which was a mistake I made last time I attempted this cleanup), as Nalu requires the compilers to be set to Trilinos' before the call to project(Nalu). Then I tried to make the finding of the TPL packages more consistent by using the files CMake installs with each package to use find_package which means for YAML we are using find_package and not find_library as the others do. I also took the liberty of trying to make the CMakeLists more consistent in style and chose lowercasing the commands and making the general formatting more consistent.

I have tested the OpenFAST test on both Linux (Peregrine), and OSX (my laptop), and the test works as I expect and runs successfully. Regarding the searches for TPLs, I have tested this on both Linux (Peregrine), and OSX (my laptop) using both shared and static libraries for OpenFAST, TIOGA, and HYPRE, leaving Trilinos static. On OSX I found that I couldn't built OpenFAST as a shared library and HYPRE does not appear to be able to build as shared on OSX as well. I was able to build YAML and TIOGA as shared, and YAML works fine, but TIOGA may not be adding the full path for itself during linking in Nalu which I have discussed with @sayerhs already, but I feel this does not impact this pull request as I have manually edited the path to the TIOGA shared library in naluX and tested successfully. On Linux, everything seemed to work as expected regardless of whether I built the TPLs as shared or static and Nalu was able to find, link and run the tests with them.

I think I would like some assurance from at least one reviewer that this at least does not break the status quo for them by testing they can still build against static libraries most importantly.

CMakeLists.txt Outdated
set(CMAKE_C_COMPILER ${Trilinos_C_COMPILER} )
set(CMAKE_Fortran_COMPILER ${Trilinos_Fortran_COMPILER} )
# Enable Fortran if it is enabled in Trilinos
if(CMAKE_Fortran_COMPILER)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be moved to project(Nalu CXX Fortran) ? MasterElements still require the fortran file, so we would require Fortran compiler even if Trilinos is not enabling it? And with ENABLE_OPENFAST we definitely need Fortran for DISCON.f90

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, right, I didn't think of that. We probably should move to project(Nalu CXX Fortran). I will do that and do a quick test on it.

@sayerhs
Copy link
Contributor

sayerhs commented Jan 10, 2018

Combined @jrood-nrel 's branch and it compiles and runs with static libraries for TIOGA and HYPRE on OSX.

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.

None yet

3 participants