Merge pull request #85 from EXP-code/pybind11fix

Proposed fix to improve pybind11 dynamic type casting

.github/workflows/build.yml

@@ -12,21 +12,29 @@ jobs:
   exp:
     strategy:
       matrix:
-        os: [ubuntu-latest]
-        cc: [gcc]
+        os: [ubuntu-24.04, ubuntu-22.04]
+        cc: [gcc, clang]
+        cxx: [g++, clang++]
+        include:
+          - cxx: clang++
+            cxxflags: -stdlib=libstdc++
+        exclude:
+          - cc: gcc
+            cxx: clang++
+          - cc: clang
+            cxx: g++
 
     name: "Test pyEXP Build"
     runs-on: ${{ matrix.os }}
     steps:
       - name: Checkout
-        uses: actions/checkout@v3
+        uses: actions/checkout@v4
 
       - name: Install core dependencies - ubuntu
         if: runner.os == 'Linux'
         run: |
           sudo apt-get update
-          sudo apt-get install -y build-essential libeigen3-dev libfftw3-dev libhdf5-dev libopenmpi-dev
-          sudo pip install numpy
+          sudo apt-get install -y build-essential libeigen3-dev libfftw3-dev libhdf5-dev libopenmpi-dev libomp-dev python3-numpy
 
       - name: Setup submodule and build
         run: |
@@ -37,6 +45,7 @@ jobs:
         if: runner.os == 'Linux'
         env:
           CC: ${{ matrix.cc }}
+          CXX: ${{ matrix.cxx }}
         working-directory: ./build
         run: >-
           cmake
@@ -45,6 +54,7 @@ jobs:
           -DCMAKE_BUILD_TYPE=Release
           -DEigen3_DIR=/usr/include/eigen3/share/eigen3/cmake
           -DCMAKE_INSTALL_PREFIX=./install
+          -DCMAKE_CXX_FLAGS=${{ matrix.cxxflags }}
           -Wno-dev
           ..
 
@@ -58,4 +68,4 @@ jobs:
 
       #- name: CTest Long
       #working-directory: ./build
-      #run: ctest -L long
+      #run: ctest --output-on-failure -L long

CMakeLists.txt

@@ -13,7 +13,6 @@ set(CMAKE_CXX_EXTENSIONS OFF)
 # Compiler flags.  Not all tested thoroughly...
 if(CMAKE_CXX_COMPILER_ID MATCHES "Clang")
   # using Clang
-  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -stdlib=libc++")
 elseif(CMAKE_CXX_COMPILER_ID MATCHES "GNU")
   # using GCC
 elseif(CMAKE_CXX_COMPILER_ID MATCHES "Intel")
@@ -63,7 +62,7 @@ set(CMAKE_FIND_PACKAGE_SORT_DIRECTION DEC)
 
 find_package(MPI REQUIRED COMPONENTS C CXX)
 find_package(OpenMP)
-find_package(FFTW)
+find_package(FFTW REQUIRED)
 find_package(HDF5 COMPONENTS C CXX HL REQUIRED)
 find_package(TIRPC)	       # Check for alternative Sun rpc support
 find_package(Eigen3 REQUIRED)

expui/BiorthBasis.cc

@@ -368,7 +368,7 @@ namespace BasisClasses
   {
     // Sanity check on derived class type
     //
-    if (typeid(*coef) != typeid(CoefClasses::SphStruct))
+    if (not dynamic_cast<CoefClasses::SphStruct*>(coef.get()))
       throw std::runtime_error("Spherical::set_coefs: you must pass a CoefClasses::SphStruct");
 
     // Sanity check on dimensionality
@@ -1431,7 +1431,7 @@ namespace BasisClasses
 
   void Cylindrical::set_coefs(CoefClasses::CoefStrPtr coef)
   {
-    if (typeid(*coef) != typeid(CoefClasses::CylStruct))
+    if (not dynamic_cast<CoefClasses::CylStruct*>(coef.get()))
       throw std::runtime_error("Cylindrical::set_coefs: you must pass a CoefClasses::CylStruct");
 
     CoefClasses::CylStruct* cf = dynamic_cast<CoefClasses::CylStruct*>(coef.get());
@@ -1708,7 +1708,7 @@ namespace BasisClasses
   {
     // Sanity check on derived class type
     //
-    if (typeid(*coef) != typeid(CoefClasses::CylStruct))
+    if (not dynamic_cast<CoefClasses::CylStruct*>(coef.get()))
       throw std::runtime_error("FlatDisk::set_coefs: you must pass a CoefClasses::CylStruct");
 
     // Sanity check on dimensionality
@@ -2140,7 +2140,7 @@ namespace BasisClasses
   {
     // Sanity check on derived class type
     //
-    if (typeid(*coef) != typeid(CoefClasses::SlabStruct))
+    if (not dynamic_cast<CoefClasses::SlabStruct*>(coef.get()))
       throw std::runtime_error("Slab::set_coefs: you must pass a CoefClasses::SlabStruct");
 
     // Sanity check on dimensionality
@@ -2572,7 +2572,7 @@ namespace BasisClasses
   {
     // Sanity check on derived class type
     //
-    if (typeid(*coef) != typeid(CoefClasses::CubeStruct))
+    if (not dynamic_cast<CoefClasses::CubeStruct*>(coef.get()))
       throw std::runtime_error("Cube::set_coefs: you must pass a CoefClasses::CubeStruct");
 
     // Sanity check on dimensionality

expui/CoefContainer.cc

@@ -72,7 +72,7 @@ namespace MSSA
       unpack_cylfld();
     }
     else {
-      throw std::runtime_error(std::string("CoefDB::unpack_channels(): can not reflect coefficient type=") + typeid(*coefs.get()).name());
+      throw std::runtime_error(std::string("CoefDB::unpack_channels(): can not reflect coefficient type=") + typeid(coefs.get()).name());
     }
 
     return coefs;
@@ -91,7 +91,7 @@ namespace MSSA
     else if (dynamic_cast<CoefClasses::CylFldCoefs*>(coefs.get()))
       { } // Do nothing
     else {
-      throw std::runtime_error(std::string("CoefDB::background(): can not reflect coefficient type=") + typeid(*coefs.get()).name());
+      throw std::runtime_error(std::string("CoefDB::background(): can not reflect coefficient type=") + typeid(coefs.get()).name());
     }
   }
 
@@ -112,7 +112,7 @@ namespace MSSA
     else if (dynamic_cast<CoefClasses::CylFldCoefs*>(coefs.get()))
       pack_cylfld();
     else {
-      throw std::runtime_error(std::string("CoefDB::pack_channels(): can not reflect coefficient type=") + typeid(*coefs.get()).name());
+      throw std::runtime_error(std::string("CoefDB::pack_channels(): can not reflect coefficient type=") + typeid(coefs.get()).name());
     }
   }
 

expui/Coefficients.H

@@ -111,7 +111,7 @@ namespace CoefClasses
     virtual Eigen::VectorXcd& getData(double time) = 0;
 
     //! Set coefficient store at given time to the provided matrix
-    virtual void setData(double time, const Eigen::VectorXcd& data) = 0;
+    virtual void setData(double time, Eigen::VectorXcd& data) = 0;
 
     //! Interpolate coefficient matrix at given time
     std::tuple<Eigen::VectorXcd&, bool> interpolate(double time);
@@ -273,10 +273,10 @@ namespace CoefClasses
     Eigen::MatrixXcd& getMatrix(double time);
 
     //! Set coefficient matrix at given time
-    virtual void setData(double time, const Eigen::VectorXcd& arr);
+    virtual void setData(double time, Eigen::VectorXcd& arr);
 
     //! Natural data setter for pyEXP
-    void setMatrix(double time, const Eigen::MatrixXcd& mat);
+    void setMatrix(double time, Eigen::MatrixXcd& mat);
 
     //! Interpolate coefficient matrix at given time
     std::tuple<Eigen::MatrixXcd&, bool> interpolate(double time)
@@ -411,10 +411,10 @@ namespace CoefClasses
     Eigen::MatrixXcd& getMatrix(double time);
 
     //! Set coefficient matrix at given time
-    virtual void setData(double time, const Eigen::VectorXcd& arr);
+    virtual void setData(double time, Eigen::VectorXcd& arr);
 
     //! For pyEXP
-    void setMatrix(double time, const Eigen::MatrixXcd& arr);
+    void setMatrix(double time, Eigen::MatrixXcd& arr);
 
     //! Interpolate coefficient matrix at given time
     std::tuple<Eigen::MatrixXcd&, bool> interpolate(double time)
@@ -542,7 +542,7 @@ namespace CoefClasses
     virtual Eigen3d& getTensor(double time);
 
     //! Set coefficient matrix at given time
-    virtual void setData(double time, const Eigen::VectorXcd& dat);
+    virtual void setData(double time, Eigen::VectorXcd& dat);
 
     //! Native version
     virtual void setTensor(double time, const Eigen3d& dat);
@@ -673,7 +673,7 @@ namespace CoefClasses
     virtual Eigen3d& getTensor(double time);
 
     //! Set coefficient matrix at given time
-    virtual void setData(double time, const Eigen::VectorXcd& dat);
+    virtual void setData(double time, Eigen::VectorXcd& dat);
 
     //! Native version
     virtual void setTensor(double time, const Eigen3d& dat);
@@ -810,9 +810,10 @@ namespace CoefClasses
     /** Set coefficient matrix at given time.  This is for pybind11,
 	since the operator() will not allow lvalue assignment, it
 	seems. */
-    virtual void setData(double time, const Eigen::VectorXcd& arr);
+    virtual void setData(double time, Eigen::VectorXcd& arr);
 
-    void setArray(double time, const Eigen::VectorXcd& arr) { setData(time, arr); }
+    void setArray(double time, Eigen::VectorXcd& arr)
+    { setData(time, arr); }
 
     //! Get coefficient structure at a given time
     virtual std::shared_ptr<CoefStruct> getCoefStruct(double time)
@@ -911,13 +912,13 @@ namespace CoefClasses
     virtual Eigen::VectorXcd& getData(double time);
 
     //! Operator for pyEXP
-    SphFldStruct::coefType& getMatrix(double time);
+    SphFldStruct::dataType getMatrix(double time);
 
     //! Set coefficient matrix at given time
-    virtual void setData(double time, const Eigen::VectorXcd& arr);
+    virtual void setData(double time, Eigen::VectorXcd& arr);
 
     //! Natural data setter for pyEXP
-    void setMatrix(double time, const SphFldStruct::coefType& mat);
+    void setMatrix(double time, SphFldStruct::dataType& mat);
 
     //! Interpolate coefficient matrix at given time
     std::tuple<SphFldStruct::coefType&, bool> interpolate(double time)
@@ -1038,13 +1039,13 @@ namespace CoefClasses
     virtual Eigen::VectorXcd& getData(double time);
 
     //! Operator for pyEXP
-    CylFldStruct::coefType & getMatrix(double time);
+    CylFldStruct::dataType getMatrix(double time);
 
     //! Set coefficient matrix at given time
-    virtual void setData(double time, const Eigen::VectorXcd& arr);
+    virtual void setData(double time, Eigen::VectorXcd& arr);
 
     //! Natural data setter for pyEXP
-    void setMatrix(double time, const CylFldStruct::coefType& mat);
+    void setMatrix(double time, CylFldStruct::dataType& mat);
 
     //! Interpolate coefficient matrix at given time
     std::tuple<CylFldStruct::coefType&, bool> interpolate(double time)