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

Improve management of TPL dependencies with new TriBITS (#299, #63) #494

Closed
bartlettroscoe opened this issue Jul 12, 2022 · 3 comments
Closed

Comments

@bartlettroscoe
Copy link
Member

It just occurred to me a better way of dealing with the break in backward compatibility with the usage of <tplName>_LIB_ENABLED_DEPENDENCIES mentioned in trilinos/Trilinos#10614 (comment) and documented in:

and in the entry "For now, dependencies between external packages (TPLs) can be set in the /TPLsList.cmake file by setting the cache var _LIB_ENABLED_DEPENDENCIES for each downstream external package (TPL). (See TribitsExampleProject2/TPLsList.cmake for an example.)" in:

The idea would be to instead of directly setting <tplName>_LIB_ENABLED_DEPENDENCIES we instead define a function tribits_external_package_define_dependencies() that sets <tplName>_LIB_ALL_DEPENDENCIES . We then add enable/disable logic in TriBITS to set the cache var <tplName>_LIB_ENABLED_DEPENDENCIES based on <tplName>_LIB_ALL_DEPENDENCIES and what TPLs are actually enabled and based on the set value of <tplName>_ENABLE_<upstreamTpl> (which would be empty by default but would allow the user to set <tplName>_ENABLE_<upstreamTpl>=OFF and disable that dependency). That should maintain perfect backward compatibility such as in cases where the TPLs are currently enable dir disabled. In the future, we will want to enable upstream TPLs by default when downstream TPLs are enabled.

@bartlettroscoe bartlettroscoe added this to Selected in TriBITS Refactor Jul 12, 2022
@bartlettroscoe bartlettroscoe moved this from Selected to In Progress in TriBITS Refactor Jul 12, 2022
@bartlettroscoe
Copy link
Member Author

bartlettroscoe commented Jul 12, 2022

CC: @keitat

So this issue with ParMETIS and METIS is actually impacting the Trilinos PR builds as shown here with the configure error like shown here showing:

...

Final set of enabled TPLs:  Pthread MPI BLAS LAPACK Boost Scotch ParMETIS Zlib HDF5 Netcdf SuperLUDist BoostLib DLlib 13

...

Processing enabled TPL: ParMETIS (enabled explicitly, disable with -DTPL_ENABLE_ParMETIS=OFF)
-- ParMETIS_LIBRARY_NAMES='parmetis;metis'
-- Searching for libs in ParMETIS_LIBRARY_DIRS='/projects/sems/install/rhel7-x86_64/sems/tpl/parmetis/4.0.3/intel/17.0.1/mpich/3.2/32bit_parallel/lib'
-- Searching for a lib in the set "parmetis":
--   Searching for lib 'parmetis' ...
--     Found lib '/projects/sems/install/rhel7-x86_64/sems/tpl/parmetis/4.0.3/intel/17.0.1/mpich/3.2/32bit_parallel/lib/libparmetis.a'
-- Searching for a lib in the set "metis":
--   Searching for lib 'metis' ...
--     Found lib '/projects/sems/install/rhel7-x86_64/sems/tpl/parmetis/4.0.3/intel/17.0.1/mpich/3.2/32bit_parallel/lib/libmetis.a'
-- TPL_ParMETIS_LIBRARIES='/projects/sems/install/rhel7-x86_64/sems/tpl/parmetis/4.0.3/intel/17.0.1/mpich/3.2/32bit_parallel/lib/libparmetis.a;/projects/sems/install/rhel7-x86_64/sems/tpl/parmetis/4.0.3/intel/17.0.1/mpich/3.2/32bit_parallel/lib/libmetis.a'
-- Searching for headers in ParMETIS_INCLUDE_DIRS='/projects/sems/install/rhel7-x86_64/sems/tpl/parmetis/4.0.3/intel/17.0.1/mpich/3.2/32bit_parallel/include'
-- Searching for a header file in the set "parmetis.h":
--   Searching for header 'parmetis.h' ...
--     Found header '/projects/sems/install/rhel7-x86_64/sems/tpl/parmetis/4.0.3/intel/17.0.1/mpich/3.2/32bit_parallel/include/parmetis.h'
-- Searching for a header file in the set "metis.h":
--   Searching for header 'metis.h' ...
--     Found header '/projects/sems/install/rhel7-x86_64/sems/tpl/parmetis/4.0.3/intel/17.0.1/mpich/3.2/32bit_parallel/include/metis.h'
-- Found TPL 'ParMETIS' include dirs '/projects/sems/install/rhel7-x86_64/sems/tpl/parmetis/4.0.3/intel/17.0.1/mpich/3.2/32bit_parallel/include'
-- TPL_ParMETIS_INCLUDE_DIRS='/projects/sems/install/rhel7-x86_64/sems/tpl/parmetis/4.0.3/intel/17.0.1/mpich/3.2/32bit_parallel/include'
CMake Error at cmake/tribits/core/package_arch/TribitsExternalPackageWriteConfigFile.cmake:331 (message):
  ERROR: METIS_DIR is empty!
Call Stack (most recent call first):
  cmake/tribits/core/package_arch/TribitsExternalPackageWriteConfigFile.cmake:266 (tribits_external_package_add_find_upstream_dependencies_str)
  cmake/tribits/core/package_arch/TribitsExternalPackageWriteConfigFile.cmake:69 (tribits_external_package_write_config_file_str)
  cmake/tribits/core/package_arch/TribitsTplFindIncludeDirsAndLibraries.cmake:721 (tribits_external_package_write_config_file)
  cmake/TPLs/FindTPLParMETIS.cmake:57 (TRIBITS_TPL_FIND_INCLUDE_DIRS_AND_LIBRARIES)
  cmake/tribits/core/package_arch/TribitsProcessEnabledTpl.cmake:100 (include)
  cmake/tribits/core/package_arch/TribitsGlobalMacros.cmake:1445 (tribits_process_enabled_tpl)
  cmake/tribits/core/package_arch/TribitsProjectImpl.cmake:196 (tribits_process_enabled_tpls)
  cmake/tribits/core/package_arch/TribitsProject.cmake:92 (tribits_project_impl)
  CMakeLists.txt:109 (TRIBITS_PROJECT)


-- Configuring incomplete, errors occurred!

...

What you see above is that these PR builds are enabling ParMETIS but not METIS and that is what causes the error.

So it is critical that I address this as above.

bartlettroscoe added a commit to bartlettroscoe/TriBITS that referenced this issue Jul 14, 2022
bartlettroscoe added a commit to bartlettroscoe/TriBITS that referenced this issue Jul 14, 2022
…b#63, TriBITSPub#299, TriBITSPub#494)

Here is what I did in detail:

* Created new module TribitsPackageDependencies.cmake (and unit test driver)
  to collect standard dependency defintion macros and functions into.

* Created function tribits_external_package_define_dependencies() that sets
  <tplName>_LIB_ALL_DEPENDENCIES and
  tribits_external_package_setup_enabled_dependencies() that sets
  <tplName>_LIB_ENABLED_DEPENDENCIES based on what TPLs are actually enabled.
  (This should address TriBITSPub#494.)

* Renamed function
  tribits_external_package_append_upstream_target_link_libraries_get_name_and_vis()
  to tribits_external_package_get_dep_name_and_vis() and moved from
  TribitsExternalPackageWriteConfigFile.cmake to
  TribitsPackageDependencies.cmake.

* Improved a bit of the existing documentation.
bartlettroscoe added a commit to bartlettroscoe/TriBITS that referenced this issue Jul 14, 2022
bartlettroscoe added a commit to bartlettroscoe/TriBITS that referenced this issue Jul 15, 2022
…TriBITSPub#63, TriBITSPub#299, TriBITSPub#494)

This commit primarily adds support for FindTPL<tplName>Dependencies.cmake
files so you don't need to call tribits_external_package_define_dependencies()
from the TPLsList.cmake file (which was a terrible hack).

As part of this, I also updated the documentation related to external
packages/TPLs.

More specifically I did the following:

* Added support for FindTPL<tplName>Dependencies.cmake files which including
  adding a new TPL var <tplName>_DEPENDENCIES_FILE.  That included adding a
  new macro tribits_read_external_package_deps_files_add_to_graph() that gets
  run for each TPL during the generation of the project package dependencies
  (and added to teh call-graph for
  tribits_read_all_project_deps_files_create_deps_graph()).

* Expanded documentation on TriBITS External Packages/TPLs:

  - Properly listed out FindTPL<tplName>.cmake and
    FindTPL<tplName>Dependencies.cmake in the section 'TriBITS External
    Package/TPL Core Files'

  - Added linked documentation entries for FindTPL<tplName>.cmake and
    FindTPL<tplName>Dependencies.cmake

  - Updated list of step to add a new TPL

  - Added a section 'Creating FindTPL<tplName>.cmake file' with mini-toc to
    the subsections to make easy to navigate

  - Added MUST_FIND_ALL_LIBS to example calls of
    tribits_tpl_find_include_dirs_and_libraries()

* Refactored TribitsExampleProject2:

  - Added FindTPL<tplName>Depenencies.cmake files for Tpl2, Tpl3, and Tpl4.

  - Remove hacked calls to tribits_external_package_define_dependencies() in
    TPLsList.cmake

* Other users and maintainers guides updates:

  - Sorted items in TribitsSystemMacroFunctionDocTemplate.rst

  - Changed from 'FindTPL*.cmake' to 'FindTPL<tplName>.cmake' in the
    documentation (which I think looks better).

  - Other small documentation fixups I noticed while looking at this stuff.

* Changed over to foreach ( ... IN LISTS ... ) for a few loops.
bartlettroscoe added a commit to bartlettroscoe/TriBITS that referenced this issue Jul 15, 2022
…b#63, TriBITSPub#299, TriBITSPub#494)

There was feedback from @KyleFromKitware that this was too long and too hard
to see the prefix.
bartlettroscoe added a commit to bartlettroscoe/TriBITS that referenced this issue Jul 15, 2022
…Pub#63, TriBITSPub#299, TriBITSPub#494)

I also reformated the entry noting that TPLs must declare dependencies.
bartlettroscoe added a commit that referenced this issue Jul 15, 2022
Add support for FindTPL<tplName>Dependencies.cmake and doc updates (#63, #299, #494)
@bartlettroscoe
Copy link
Member Author

This is addressed in PR #495.

@bartlettroscoe bartlettroscoe moved this from In Progress to In Review in TriBITS Refactor Jul 15, 2022
bartlettroscoe added a commit to bartlettroscoe/Trilinos that referenced this issue Jul 15, 2022
bartlettroscoe added a commit to bartlettroscoe/Trilinos that referenced this issue Jul 15, 2022
…SPub/TriBITS#494)

Now no errors will occur when all of the upstream TPLs are not enabled.  For
exmaple, this will allow ParMETIS to be enabled but not METIS (like is done
with the Trilinos PR GenConfig builds).

I also removed item from RELEASE_NOTES on break in backward compatibility for
TPL dependencies.
jmgate pushed a commit to tcad-charon/Trilinos that referenced this issue Jul 20, 2022
…s:develop' (e7b79cb).

* trilinos-develop: (50 commits)
  Add rowmap import
  Update .gitdist.default for tribits_github_snapshot worktree
  Automatic snapshot commit from tribits at 0ac69f49
  Correctly handle dependencies for SuperLUDist (TriBITSPub/TriBITS#299)
  Give SuperLU TPL proper dependencies (TriBITSPub/TriBITS#299)
  Ignore locally generated *.ini
  Switch over to using FindTPL<tplName>Dependencies.cmake files (TriBITSPub/TriBITS#494)
  Automatic snapshot commit from tribits at e121d767
  Add C++17 standard for GCC 7.2.0 and GCC 7.2.0 serial
  Fix formatting
  Add C++17 standard for Clang 10.0.0 PR build
  FastILU : sort only not Metis, and sorted Tpetra CrsMatrix
  FastILU : replace create_mirror with create_mirror_view
  FastILU : sort only not Metis
  FastILU : create a map from (sorted) A to (sorted) U during symbolic, to avoid sorting of U in compute
  FastILU : skip sorting only if Metis is not requested.
  Add release note on needing to set <tplName>_LIB_ENABLED_DEPENDENCIES and issues (TriBITSPub/TriBITS#299)
  Automatic snapshot commit from tribits at c81102e2
  Ifpack2 : fix Kokkos_ENABLE_DEBUG_BOUNDS_CHECK message, and disable skip-sort
  Ifpack2 : enable skip-sort option
  ...
jmgate pushed a commit to tcad-charon/Trilinos that referenced this issue Jul 20, 2022
…s:develop' (e7b79cb).

* trilinos-develop: (50 commits)
  Add rowmap import
  Update .gitdist.default for tribits_github_snapshot worktree
  Automatic snapshot commit from tribits at 0ac69f49
  Correctly handle dependencies for SuperLUDist (TriBITSPub/TriBITS#299)
  Give SuperLU TPL proper dependencies (TriBITSPub/TriBITS#299)
  Ignore locally generated *.ini
  Switch over to using FindTPL<tplName>Dependencies.cmake files (TriBITSPub/TriBITS#494)
  Automatic snapshot commit from tribits at e121d767
  Add C++17 standard for GCC 7.2.0 and GCC 7.2.0 serial
  Fix formatting
  Add C++17 standard for Clang 10.0.0 PR build
  FastILU : sort only not Metis, and sorted Tpetra CrsMatrix
  FastILU : replace create_mirror with create_mirror_view
  FastILU : sort only not Metis
  FastILU : create a map from (sorted) A to (sorted) U during symbolic, to avoid sorting of U in compute
  FastILU : skip sorting only if Metis is not requested.
  Add release note on needing to set <tplName>_LIB_ENABLED_DEPENDENCIES and issues (TriBITSPub/TriBITS#299)
  Automatic snapshot commit from tribits at c81102e2
  Ifpack2 : fix Kokkos_ENABLE_DEBUG_BOUNDS_CHECK message, and disable skip-sort
  Ifpack2 : enable skip-sort option
  ...
@bartlettroscoe
Copy link
Member Author

This has not caused any problems after the merge to Trilinos 'develop' and in fact has helped to make it easy to resolve issues with TPL dependencies in trilinos/Trilinos#10614.

TriBITS Refactor automation moved this from In Review to Done Jul 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

No branches or pull requests

1 participant