Intrelab: Switch to target based Intrelab CMake build #10631

kuberry · 2022-06-17T16:04:17Z

@trilinos/intrepid

Motivation

Switch to the modern target based CMake for building mex files.

kuberry · 2022-06-24T19:35:01Z

@bartlettroscoe Once this PR is finished (switching to target based CMake for Intrelab), will we want to have it merged prior to the TriBITS update or with it (or even after)?

bartlettroscoe · 2022-06-24T20:31:51Z

@bartlettroscoe Once this PR is finished (switching to target based CMake for Intrelab), will we want to have it merged prior to the TriBITS update or with it (or even after)?

@kuberry, these changes should be merged along with or after PR #10614 is merged. (There are few remaining items to address before that can be merged, likely mid to late next week.) I could have them merged at the same time by merging this topic branch into the topic branch for PR #10614 and that would result in them being merged at the same time. That is likely the best option.

Just tell me when this branch is ready to merge and I will merge it into the branch associated with PR #10614.

bartlettroscoe · 2022-06-27T16:58:42Z

@kuberry, I see you removed the copied and modified function matlab_add_mex_mac(). Is this branch now working along with the TriBITS upgrade in PR #10614? Is this PR's topic branch kuberry:intrelab_trilinos_14 ready to merge into bartlettroscoe:tribits-299-modern-cmake-targets-1-again-2 so it gets merged at the same time that PR #10614 gets merged?

kuberry · 2022-06-27T20:33:06Z

@bartlettroscoe Yes, this branch is working along with #10614. I'm waiting to hear back from @dridzal, but I believe it is ready to be merged.

dridzal

Intrelab builds and tests pass on MacOS, with cmake 3.22.1. On Linux, there are errors:

CMake Error at /net/sherlock.sandia.gov/storage/fast/projects/sems/install/rhel7-x86_64/sems/v2/utility/cmake/3.22.4/gcc/8.3.0/base/iyazzmq/share/cmake-3.22/Modules/FindPackageHandleStandardArgs.cmake:230 (message):
  Could NOT find Matlab (missing: Matlab_INCLUDE_DIRS) (found version
  "NOTFOUND")
Call Stack (most recent call first):
  /net/sherlock.sandia.gov/storage/fast/projects/sems/install/rhel7-x86_64/sems/v2/utility/cmake/3.22.4/gcc/8.3.0/base/iyazzmq/share/cmake-3.22/Modules/FindPackageHandleStandardArgs.cmake:594 (_FPHSA_FAILURE_MESSAGE)
  /net/sherlock.sandia.gov/storage/fast/projects/sems/install/rhel7-x86_64/sems/v2/utility/cmake/3.22.4/gcc/8.3.0/base/iyazzmq/share/cmake-3.22/Modules/FindMatlab.cmake:1946 (find_package_handle_standard_args)
  CMakeLists.txt:30 (find_package)

I looked at the cmake cache file, and it reports:

//Matlab installation root path
Matlab_ROOT_DIR:PATH=/net/sherlock.sandia.gov/storage/fast/projects/sems/install/rhel7-x86_64/sems/utility/tex/2019/bin

This is clearly wrong (why did it choose this??). I specifically set

set(MATLAB_ROOT_DIR "/home/dridzal/software/MATLAB/R2020b/")
if(DEFINED ENV{Matlab_ROOT_DIR})
  set(Matlab_ROOT_DIR $ENV{Matlab_ROOT_DIR})
endif()
find_package(Matlab REQUIRED)

I have no idea why cmake is ignoring the variable I set.

bartlettroscoe · 2022-06-28T21:31:13Z

@dridzal, did you mean to set:

set(MATLAB_ROOT_DIR "/home/dridzal/software/MATLAB/R2020b/")

instead of:

set(Matlab_ROOT_DIR "/home/dridzal/software/MATLAB/R2020b/")

?

CMake variable names are case-sensitive.

dridzal · 2022-06-28T21:42:47Z

Thanks @bartlettroscoe . That is an issue. I'll report back soon.

dridzal · 2022-06-29T00:40:40Z

I fixed the Matlab path and built Intrelab successfully on Linux. Unfortunately, after starting Matlab as I normally do for Intrelab, I get the error:

/home/dridzal/development/Intrelab/Trilinos/packages/intrepid/matlab/intrelab/install/intrepid_getNumCubaturePoints.mexa64: undefined symbol:
_Z14m2iGetCellTopoRN7Teuchos3RCPIN6shards12CellTopologyEEERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEERSA_

To clarify, this is a Matlab mexa64 executable looking for a library function from Trilinos. This used to work with the old Trilinos and the old Intrelab CMakeLists.txt. I've tried building Trilinos in shared library mode and in static mode with fPIC. Both builds result in the same error. Adding my Trilinos /install/lib directory to the LD_LIBRARY_PATH does not help either.

kuberry · 2022-06-29T02:00:22Z

@dridzal Can you run:

ldd /home/dridzal/development/Intrelab/Trilinos/packages/intrepid/matlab/intrelab/install/intrepid_getNumCubaturePoints.mexa64

and print the output?

Also, could you run:

readelf -d ../install/intrepid_getNumCubaturePoints.mexa64

and print that output? This will inform whether this is an RPATH issue or not.

dridzal · 2022-06-29T16:41:30Z

@kuberry : The first command gives a lot of output. I see tons of "standard" unrelated libraries, but nothing at all that looks like Trilinos. The second command gives:

Dynamic section at offset 0x3b4d08 contains 33 entries:
  Tag        Type                         Name/Value
 0x0000000000000001 (NEEDED)             Shared library: [libmex.so]
 0x0000000000000001 (NEEDED)             Shared library: [libmx.so]
 0x0000000000000001 (NEEDED)             Shared library: [libMatlabEngine.so]
 0x0000000000000001 (NEEDED)             Shared library: [libstdc++.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [libgcc_s.so.1]
 0x0000000000000001 (NEEDED)             Shared library: [libc.so.6]
 0x000000000000000e (SONAME)             Library soname: [intrepid_getNumCubaturePoints.mexa64]
 0x000000000000000f (RPATH)              Library rpath: [/ascldap/users/dridzal/software/MATLAB/R2020b/bin/glnxa64:/ascldap/users/dridzal/software/MATLAB/R2020b/extern/bin/glnxa64]
 0x000000000000000c (INIT)               0x4428
 0x000000000000000d (FINI)               0x1db04
 0x0000000000000019 (INIT_ARRAY)         0x5b4590
 0x000000000000001b (INIT_ARRAYSZ)       48 (bytes)
 0x000000000000001a (FINI_ARRAY)         0x5b45c0
 0x000000000000001c (FINI_ARRAYSZ)       8 (bytes)
 0x0000000000000004 (HASH)               0x190
 0x0000000000000005 (STRTAB)             0x1058
 0x0000000000000006 (SYMTAB)             0x500
 0x000000000000000a (STRSZ)              4739 (bytes)
 0x000000000000000b (SYMENT)             24 (bytes)
 0x0000000000000003 (PLTGOT)             0x5b5000
 0x0000000000000002 (PLTRELSZ)           2232 (bytes)
 0x0000000000000014 (PLTREL)             RELA
 0x0000000000000017 (JMPREL)             0x3b70
 0x0000000000000007 (RELA)               0x24d8
 0x0000000000000008 (RELASZ)             5784 (bytes)
 0x0000000000000009 (RELAENT)            24 (bytes)
 0x000000006ffffffc (VERDEF)             0x23d0
 0x000000006ffffffd (VERDEFNUM)          2
 0x000000006ffffffe (VERNEED)            0x2408
 0x000000006fffffff (VERNEEDNUM)         3
 0x000000006ffffff0 (VERSYM)             0x22dc
 0x000000006ffffff9 (RELACOUNT)          183
 0x0000000000000000 (NULL)               0x0

What's your conclusion?

dridzal

MacOS and Linux builds work with Matlab 2020a and 2020b, respectively. I think this is ready to be merged.

kuberry · 2022-06-30T13:12:04Z

MacOS and Linux builds work with Matlab 2020a and 2020b, respectively. I think this is ready to be merged.

also 2021b

kuberry · 2022-06-30T16:28:17Z

@bartlettroscoe Hi Ross, can we get this merged into #10614?

bartlettroscoe · 2022-06-30T20:42:24Z

@bartlettroscoe Hi Ross, can we get this merged into #10614?

@kuberry, I have a checklist item to merge this at the top of #10614 before merging that PR. So this should be a smooth transition.

bartlettroscoe · 2022-06-30T20:42:58Z

So this should be a smooth transition.

At least as Intrelab is concerned :-)

bartlettroscoe

Just a few suggestions to improve this. Otherwise, this is very simple and straightforward. It is almost too simple (which is a major victory, no?).

bartlettroscoe · 2022-07-01T19:38:15Z

packages/intrepid/matlab/intrelab/CMakeLists.txt

@@ -39,159 +55,20 @@ endif()

 FIND_PACKAGE(Trilinos REQUIRED)


I would suggest that you pass in the exact Trilinos packages that Intrelab needs with:

find_package(Trilinos COMPONETS <pkg1> <plg2> ... REQUIRED)

so that if someone tries to configure this CMake project against an install of Trilinos without the necessary packages, then the find_package(Trilinos ... REQUIRED) command will error out and tell them what Trilinos packages (i.e. components) are missing. Otherwise, you will not see this mistake until build time at which point would confuse whoever is trying to builds these MEX files.

packages/intrepid/matlab/intrelab/CMakeLists.txt

- Changes from requiring CMake 3.10 to 3.12 to ensure "*_ROOT" is supported - Reformat MOBJS list - Change variable modification to work with Matlab_ROOT, MATLAB_ROOT, Matlab_ROOT_DIR, and MATLAB_ROOT_DIR as well as Trilinos_ROOT and TRILINOS_ROOT

dridzal · 2022-07-05T16:06:32Z

@bartlettroscoe : With @kuberry 's prior work on this, and your recommended changes, it looks like we are not far from building Intrelab as part of Trilinos, rather than a standalone project. This would be much simpler for end users. They would just need some way of setting the Matlab root directory. What are your thoughts on this? I'm perfectly content with the few extra steps to build it separately, however considering that Intrelab is already distributed as part of Intrepid, it may make sense to build it as part of the Trilinos build.

bartlettroscoe · 2022-07-05T16:15:43Z

it looks like we are not far from building Intrelab as part of Trilinos, rather than a standalone project.

@dridzal, you can't really build Intrelab as part of Trilinos per-say since it requires an install of Trilinos in order for find_package(Trilinos ...) to work. But you could write a very thin Trilinos TriBITS wrapper package that would build and install Intrelab, perhaps as an additional build target and/or perhaps after as part of a CTest test (which runs after the build).

I am actually going to write a Trilinos CTest test that does the install of Trilinos and another CTest test that builds and runs the example application as part of the Trilinos PR test suite. That will show how this gets done. (But first, I need to merge PR #10614.)

bartlettroscoe · 2022-07-05T16:18:00Z

But you could write a very thin Trilinos TriBITS wrapper package that would build and install Intrelab, perhaps as an additional build target and/or perhaps after as part of a CTest test (which runs after the build).

@dridzal, and actually, if there is a Matlab license on the Trilinos PR build machines, you could even use this to test the Intrelab build and perhaps run some Matlab tests that ensure that this interface layer is working. (Otherwise, you could set up a nightly build on a machine with a Matlab license so you would know within 24 hours if any Trilinos commit broke it.)

dridzal · 2022-07-05T16:33:47Z

@bartlettroscoe : Thanks for the info, that sounds very promising. A tested build against Matlab would be another important step. There are other packages, like @trilinos/muelu , that could benefit from this infrastructure. For now, let's complete this first step of modernizing the "manual" build, and then we'll attack the "fully automated" build in the next iteration.

- Adds -O3, -mtune, -march flags for Release builds

dridzal

Tests pass.

kuberry · 2022-07-06T17:24:05Z

@bartlettroscoe This is ready to go. Please feel free to merge with #10614 when you are ready.

bartlettroscoe · 2022-07-12T16:50:16Z

FYI: With the merge commit 16a8e73 to the branch https://github.com/bartlettroscoe/Trilinos/tree/tribits-299-modern-cmake-targets-1-again-2, this PR will merge automatically when the PR #10614 is merged to 'develop'.

Intrelab: Switch to target based Intrelab CMake build

0d31074

kuberry added the AT: WIP Causes the PR autotester to not test the PR. (Remove to allow testing to occur.) label Jun 17, 2022

kuberry requested a review from dridzal June 17, 2022 16:04

kuberry self-assigned this Jun 17, 2022

kuberry added 4 commits June 17, 2022 14:22

Add mac version of matlab_add_mex()

887dbb0

Make intrelab libraries installable

b14d4ec

Change comment about matlab directory

2f6d8b3

Remove custom matlab_add_mex_mac()

4e027db

kuberry added 2 commits June 24, 2022 13:49

Removed redundant CXX flag copying

0368ebf

Remove sysroot modification and warning for mac

4e000b4

bartlettroscoe mentioned this pull request Jun 27, 2022

TriBITS: Switch to modern cmake targets (TriBITSPub/TriBITS#299) #10614

Merged

16 tasks

dridzal requested changes Jun 28, 2022

View reviewed changes

Support MATLAB_ROOT_DIR (all caps) variable setting

b1afc4a

kuberry force-pushed the intrelab_trilinos_14 branch from b6273ca to b1afc4a Compare June 28, 2022 22:49

Removed m2i_helpers as a target and moved it to a source

567eb28

dridzal previously approved these changes Jun 29, 2022

View reviewed changes

kuberry added AT: WIP Causes the PR autotester to not test the PR. (Remove to allow testing to occur.) and removed AT: WIP Causes the PR autotester to not test the PR. (Remove to allow testing to occur.) labels Jun 30, 2022

bartlettroscoe reviewed Jul 1, 2022

View reviewed changes

kuberry dismissed dridzal’s stale review via b9aa9d0 July 5, 2022 13:26

Switch from Trilinos::all_libs to Trilinos::all_selected_libs

02c6880

Change required CMake version to 3.17.0 and support Trilinos_DIR

f772b2a

- Adds -O3, -mtune, -march flags for Release builds

kuberry force-pushed the intrelab_trilinos_14 branch from 6276f8e to f772b2a Compare July 5, 2022 18:04

kuberry added 3 commits July 5, 2022 18:16

Make output less verbose

46c3ba3

Change INST_DIR to a CACHE variable

9b31e86

Removed INST_DIR in favor of CMAKE_INSTALL_PREFIX

5011ab5

dridzal approved these changes Jul 6, 2022

View reviewed changes

trilinos-autotester merged commit 16a8e73 into trilinos:develop Jul 18, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Intrelab: Switch to target based Intrelab CMake build #10631

Intrelab: Switch to target based Intrelab CMake build #10631

kuberry commented Jun 17, 2022

kuberry commented Jun 24, 2022

bartlettroscoe commented Jun 24, 2022

bartlettroscoe commented Jun 27, 2022

kuberry commented Jun 27, 2022

dridzal left a comment •

edited by bartlettroscoe

Loading

bartlettroscoe commented Jun 28, 2022

dridzal commented Jun 28, 2022

dridzal commented Jun 29, 2022

kuberry commented Jun 29, 2022 •

edited

Loading

dridzal commented Jun 29, 2022 •

edited

Loading

dridzal left a comment

kuberry commented Jun 30, 2022

kuberry commented Jun 30, 2022

bartlettroscoe commented Jun 30, 2022

bartlettroscoe commented Jun 30, 2022

bartlettroscoe left a comment

bartlettroscoe Jul 1, 2022

dridzal commented Jul 5, 2022

bartlettroscoe commented Jul 5, 2022

bartlettroscoe commented Jul 5, 2022

dridzal commented Jul 5, 2022

dridzal left a comment

kuberry commented Jul 6, 2022

bartlettroscoe commented Jul 12, 2022

Intrelab: Switch to target based Intrelab CMake build #10631

Intrelab: Switch to target based Intrelab CMake build #10631

Conversation

kuberry commented Jun 17, 2022

Motivation

kuberry commented Jun 24, 2022

bartlettroscoe commented Jun 24, 2022

bartlettroscoe commented Jun 27, 2022

kuberry commented Jun 27, 2022

dridzal left a comment • edited by bartlettroscoe Loading

Choose a reason for hiding this comment

bartlettroscoe commented Jun 28, 2022

dridzal commented Jun 28, 2022

dridzal commented Jun 29, 2022

kuberry commented Jun 29, 2022 • edited Loading

dridzal commented Jun 29, 2022 • edited Loading

dridzal left a comment

Choose a reason for hiding this comment

kuberry commented Jun 30, 2022

kuberry commented Jun 30, 2022

bartlettroscoe commented Jun 30, 2022

bartlettroscoe commented Jun 30, 2022

bartlettroscoe left a comment

Choose a reason for hiding this comment

bartlettroscoe Jul 1, 2022

Choose a reason for hiding this comment

dridzal commented Jul 5, 2022

bartlettroscoe commented Jul 5, 2022

bartlettroscoe commented Jul 5, 2022

dridzal commented Jul 5, 2022

dridzal left a comment

Choose a reason for hiding this comment

kuberry commented Jul 6, 2022

bartlettroscoe commented Jul 12, 2022

dridzal left a comment •

edited by bartlettroscoe

Loading

kuberry commented Jun 29, 2022 •

edited

Loading

dridzal commented Jun 29, 2022 •

edited

Loading