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

Proposed fix to improve pybind11 dynamic type casting #85

Merged
merged 56 commits into from
Oct 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
56 commits
Select commit Hold shift + click to select a range
76cdbd5
Implementation of a point set evaluator for FieldGenerator
Aug 7, 2024
9739781
Use Eigen::Ref<> to pass arrays to prevent pybind11 mapping failure i…
Sep 26, 2024
12176dc
Change names of Eigen::Tensor-->ndarray generators for clarity
Sep 28, 2024
01516c3
Updates for setMatrix() method for field coefficient types which need…
Sep 28, 2024
14a6ef5
Added CTests for 'setMatrix()' and 'createFromArray' [no ci]
Oct 12, 2024
1a65ec6
Typo in test label
Oct 12, 2024
eb4bfd3
Use random data rather than depend on a body file
Oct 12, 2024
b81da9a
Get some verbose output from github runner
Oct 12, 2024
88d1338
Make ctest log the output on failure
Oct 12, 2024
41db618
Remove intentional failure from test script
Oct 12, 2024
b937795
Add another print statement
Oct 12, 2024
dd78429
Add another print statement
Oct 12, 2024
e9600e4
Exit after basis creation; sanity check
Oct 13, 2024
eaa6521
Exit after data creation; sanity check
Oct 13, 2024
6432520
Exit after first coef creation; sanity check
Oct 13, 2024
8d332a2
Exit after third coef creation; sanity check
Oct 13, 2024
2369691
Exit after second coef creation; sanity check
Oct 13, 2024
747e6a5
Comment out second coef creation; sanity check
Oct 13, 2024
1043436
Comment out all but first coef creation; sanity check
Oct 13, 2024
278e1fe
Restore original test script
Oct 13, 2024
856a547
Merge branch 'pybind11fix' of github.com:EXP-code/EXP into pybind11fix
Oct 13, 2024
21045a6
Label creatCoefs test as 'long' to prevent CI failure
Oct 13, 2024
4c27205
Update createCoefs.py
The9Cat Oct 13, 2024
6853072
Update createCoefs.py
The9Cat Oct 13, 2024
ca80b09
Update createCoefs.py
The9Cat Oct 13, 2024
7035069
Update createCoefs.py
The9Cat Oct 13, 2024
f2932d2
Spruce up for final commit (I hope)
Oct 13, 2024
8a3d555
Change to pass arrays by copy rather than reference
Oct 13, 2024
fd69366
Restore createArray test to 'quick'
Oct 13, 2024
34bedce
Update createCoefs.py
The9Cat Oct 13, 2024
fa3740c
Update createCoefs.py
The9Cat Oct 13, 2024
996054d
Update createCoefs.py
The9Cat Oct 13, 2024
0343410
Update createCoefs.py
The9Cat Oct 13, 2024
0fda70e
Update CMakeLists.txt
The9Cat Oct 14, 2024
42be626
Add extra checks to CI for runner diagnostic information
michael-petersen Oct 14, 2024
f9c0bac
restore coef check to quick to induce failures
michael-petersen Oct 14, 2024
bfbfbc8
Rollback to reference version of setMatrix() and createFromArray(); r…
Oct 14, 2024
d2866db
Update CMakeLists.txt
The9Cat Oct 14, 2024
a007705
git submodule updated
Oct 14, 2024
02d09fb
Relabel test as 'quick' after updating pybind11
Oct 14, 2024
b215b49
Update build.yml
The9Cat Oct 14, 2024
ac91538
Update build.yml
The9Cat Oct 15, 2024
cdcab97
Update build.yml
The9Cat Oct 15, 2024
28fa507
Update build.yml
The9Cat Oct 15, 2024
ae7fa0a
Update build.yml
The9Cat Oct 15, 2024
60ffa3d
Update CMakeLists.txt
The9Cat Oct 15, 2024
7d9cf63
Decrease of the clang chatter; add 24.04 explicitly to the CI matrix
Oct 15, 2024
136fa16
Update build.yml
The9Cat Oct 15, 2024
0218739
This commit addresses all of the warnings and non-compliance issues i…
Oct 16, 2024
755bd90
Update for 24.04: remove pip install in favor of apt install of numpy
Oct 16, 2024
e712207
custom MacOS fix for fftw_complex destructor problems
michael-petersen Oct 16, 2024
693c1f9
require FFTW
michael-petersen Oct 16, 2024
455c03f
Use reinterpret cast from std::complex to handle FFTW complex types
Oct 16, 2024
e735406
Merge branch 'ClangCompliance' of github.com:EXP-code/EXP into ClangC…
Oct 16, 2024
a806434
Merge of pointMesh; fix reflection in BiorthBasis
Oct 16, 2024
9c6a72d
Merge branch 'pybind11fix' into ClangCompliance
michael-petersen Oct 16, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 17 additions & 7 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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: |
Expand All @@ -37,6 +45,7 @@ jobs:
if: runner.os == 'Linux'
env:
CC: ${{ matrix.cc }}
CXX: ${{ matrix.cxx }}
working-directory: ./build
run: >-
cmake
Expand All @@ -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
..

Expand All @@ -54,8 +64,8 @@ jobs:

- name: CTest Quick
working-directory: ./build
run: ctest -L quick
run: ctest --output-on-failure -L quick

#- name: CTest Long
#working-directory: ./build
#run: ctest -L long
#run: ctest --output-on-failure -L long
3 changes: 1 addition & 2 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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)
Expand Down
10 changes: 5 additions & 5 deletions expui/BiorthBasis.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions expui/CoefContainer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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());
}
}

Expand All @@ -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());
}
}

Expand Down
31 changes: 16 additions & 15 deletions expui/Coefficients.H
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
Loading