search

ossimlabs / ossim

Core OSSIM (Open Source Software Image Map) package including C++ code for OSSIM library, command-line applications, tests, and build system
MIT License
295 stars 142 forks source link

Eleaver rpcmodel hdf5 #255

Closed eleaver closed 4 years ago

eleaver commented 4 years ago
  1. Made ~ossimRpcModel() dtor public rather than protected. I assume this had been an oversight?
  2. CMakeLists.txt: a. Added an explicit OPTION(BUILD_OSSIM_HDF5_SUPPORT) that may be set from command line b. To improve MPI support, added include_directories( ${MPI_CXX_INCLUDE_PATH} ${MPI_CXX_INCLUDE_DIRS} ) include_directories( ${MPI_C_INCLUDE_PATH} ${MPI_C_INCLUDE_DIRS} ) These are in addition to the extant include_directories( ${MPI_INCLUDE_DIR} ) MPI_INCLUDE_DIR is not set by my /usr/share/cmake/Modules/FindMPI.cmake, but might be by someone else's. c. Added some general CMake boilerplate to enable RPATH support by default. I tripped over this when I configured ossim with MPI and tried to load a shared library from a client application. It was easier to add RPATH to ossim's CMakeLists.txt once, than try to maintain disparate client LD_LIBRARY_PATHs in perpetuity; I hope you can accept it.

I've tested this build on Fedora-32, CentOS-7, and CentOS-8. a. Fedora-32 and CentOS-8's /usr/share/cmake/Modules/FindMPI.cmake set MPI_C_INCLUDE_DIRS, while CentOS-7 sets MPI_C_INCLUDE_PATH. Same for CXX. b. HDF5 doesn't build on CentOS-7 as it's /usr/include/H5Object does not define getObjName(). c. Otherwise CentOS-7 builds fine, with the caveate that on CentOS-7 one might need to manually edit /usr/include/jsoncpp/json/value.h and change "bool operator!() const;" to "bool operator!() const { return isNull(); };" -- at least with epel jsoncpp-devel version 0.10.5.

I've attached my configuration script, ConfigureOssim.sh.txt ConfigureOssim.sh.txt

gpotts commented 4 years ago

Hello Ed:

I am in the middle of planing meetings today. Will have to look at the pull request tomorrow.

Thank you !

Take care

Garrett

On May 21, 2020, at 1:27 PM, Ed Leaver notifications@github.com wrote:

Made ~ossimRpcModel() dtor public rather than protected. I assume this had been an oversight? CMakeLists.txt: a. Added an explicit OPTION(BUILD_OSSIM_HDF5_SUPPORT) that may be set from command line b. To improve MPI support, added include_directories( ${MPI_CXX_INCLUDE_PATH} ${MPI_CXX_INCLUDE_DIRS} ) include_directories( ${MPI_C_INCLUDE_PATH} ${MPI_C_INCLUDE_DIRS} ) These are in addition to the extant include_directories( ${MPI_INCLUDE_DIR} ) MPI_INCLUDE_DIR is not set by my /usr/share/cmake/Modules/FindMPI.cmake, but might be by someone else's. c. Added some general CMake boilerplate to enable RPATH support by default. I tripped over this when I configured ossim with MPI and tried to load a shared library from a client application. It was easier to add RPATH to ossim's CMakeLists.txt once, than try to maintain disparate client LD_LIBRARY_PATHs in perpetuity; I hope you can accept it. I've tested this build on Fedora-32, CentOS-7, and CentOS-8. a. Fedora-32 and CentOS-8's /usr/share/cmake/Modules/FindMPI.cmake set MPI_C_INCLUDE_DIRS, while CentOS-7 sets MPI_C_INCLUDE_PATH. Same for CXX. b. HDF5 doesn't build on CentOS-7 as it's /usr/include/H5Object does not define getObjName(). c. Otherwise CentOS-7 builds fine, with the caveate that on CentOS-7 one might need to manually edit /usr/include/jsoncpp/json/value.h and change "bool operator!() const;" to "bool operator!() const { return isNull(); };" -- at least with epel jsoncpp-devel version 0.10.5.

I've attached my configuration script, ConfigureOssim.sh.txt ConfigureOssim.sh.txt https://github.com/ossimlabs/ossim/files/4663813/ConfigureOssim.sh.txt You can view, comment on, or merge this pull request online at:

https://github.com/ossimlabs/ossim/pull/255 https://github.com/ossimlabs/ossim/pull/255 Commit Summary

  1. Changed ossimRpcModel's destructor to public, rather than protected. Merge branch 'dev' into eleaver-rpcmodel-hdf5 include_directories( ${MPI_CXX_INCLUDE_PATH} ${MPI_C_INCLUDE_PATH} ) to satisfy FindMPI.cmake on CentOS-7 File Changes

M CMakeLists.txt https://github.com/ossimlabs/ossim/pull/255/files#diff-af3b638bc2a3e6c650974192a53c7291 (30) M include/ossim/projection/ossimRpcModel.h https://github.com/ossimlabs/ossim/pull/255/files#diff-e5480c3628baa6e244b082405c75644b (6) Patch Links:

https://github.com/ossimlabs/ossim/pull/255.patch https://github.com/ossimlabs/ossim/pull/255.patch https://github.com/ossimlabs/ossim/pull/255.diff https://github.com/ossimlabs/ossim/pull/255.diff — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/ossimlabs/ossim/pull/255, or unsubscribe https://github.com/notifications/unsubscribe-auth/AARP3PYKYGP5MAHDP4JPX7TRSVP7VANCNFSM4NHALKBQ.

eleaver commented 4 years ago

I've now built this PR on Ubuntu 18.04 using MPICH. An updated ConfigureOssim.sh is attached. ConfigureOssim.sh.txt

gpotts commented 4 years ago

Hello Ed. Yea, originally just ran out of time. The destructor was hidden for we were not going to allow any explicit deletes but instead use referenced pointers. I wanted to migrate our old ossimRefPtr to use the new standards but that rippled and never had a chance to migrate. We will keep the move of the destructor to public until we can migrate over to an official ref pointer standard.

gpotts commented 4 years ago

Thank you for the pull request. It looks good and will merge everything into the baseline.

gpotts commented 4 years ago

Hello Ed:

I might capture your build config so we can have a saved example for the MPICH versus OpenMpi configuration. Thank you for the sample configure for these options! My put it under our template directory or in a README.md or something.

Take care

Garrett

On May 21, 2020, at 1:27 PM, Ed Leaver notifications@github.com wrote:

Made ~ossimRpcModel() dtor public rather than protected. I assume this had been an oversight? CMakeLists.txt: a. Added an explicit OPTION(BUILD_OSSIM_HDF5_SUPPORT) that may be set from command line b. To improve MPI support, added include_directories( ${MPI_CXX_INCLUDE_PATH} ${MPI_CXX_INCLUDE_DIRS} ) include_directories( ${MPI_C_INCLUDE_PATH} ${MPI_C_INCLUDE_DIRS} ) These are in addition to the extant include_directories( ${MPI_INCLUDE_DIR} ) MPI_INCLUDE_DIR is not set by my /usr/share/cmake/Modules/FindMPI.cmake, but might be by someone else's. c. Added some general CMake boilerplate to enable RPATH support by default. I tripped over this when I configured ossim with MPI and tried to load a shared library from a client application. It was easier to add RPATH to ossim's CMakeLists.txt once, than try to maintain disparate client LD_LIBRARY_PATHs in perpetuity; I hope you can accept it. I've tested this build on Fedora-32, CentOS-7, and CentOS-8. a. Fedora-32 and CentOS-8's /usr/share/cmake/Modules/FindMPI.cmake set MPI_C_INCLUDE_DIRS, while CentOS-7 sets MPI_C_INCLUDE_PATH. Same for CXX. b. HDF5 doesn't build on CentOS-7 as it's /usr/include/H5Object does not define getObjName(). c. Otherwise CentOS-7 builds fine, with the caveate that on CentOS-7 one might need to manually edit /usr/include/jsoncpp/json/value.h and change "bool operator!() const;" to "bool operator!() const { return isNull(); };" -- at least with epel jsoncpp-devel version 0.10.5.

I've attached my configuration script, ConfigureOssim.sh.txt ConfigureOssim.sh.txt https://github.com/ossimlabs/ossim/files/4663813/ConfigureOssim.sh.txt You can view, comment on, or merge this pull request online at:

https://github.com/ossimlabs/ossim/pull/255 https://github.com/ossimlabs/ossim/pull/255 Commit Summary

  1. Changed ossimRpcModel's destructor to public, rather than protected. Merge branch 'dev' into eleaver-rpcmodel-hdf5 include_directories( ${MPI_CXX_INCLUDE_PATH} ${MPI_C_INCLUDE_PATH} ) to satisfy FindMPI.cmake on CentOS-7 File Changes

M CMakeLists.txt https://github.com/ossimlabs/ossim/pull/255/files#diff-af3b638bc2a3e6c650974192a53c7291 (30) M include/ossim/projection/ossimRpcModel.h https://github.com/ossimlabs/ossim/pull/255/files#diff-e5480c3628baa6e244b082405c75644b (6) Patch Links:

https://github.com/ossimlabs/ossim/pull/255.patch https://github.com/ossimlabs/ossim/pull/255.patch https://github.com/ossimlabs/ossim/pull/255.diff https://github.com/ossimlabs/ossim/pull/255.diff — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/ossimlabs/ossim/pull/255, or unsubscribe https://github.com/notifications/unsubscribe-auth/AARP3PYKYGP5MAHDP4JPX7TRSVP7VANCNFSM4NHALKBQ.