Chaste / Chaste

Chaste - Cancer Heart And Soft Tissue Environment - main public repository.
https://chaste.github.io/
Other
118 stars 55 forks source link

Broken interaction between std::vector and ublas::c_vector #280

Open fcooper8472 opened 1 month ago

fcooper8472 commented 1 month ago

This issue documents a specific problematic interaction as of GCC 13.x between std::vector and ublas::c_vector, specifically when the c_vector is one-dimensional.

When compiling in Release mode, there is a failure during compilation of ExtendedBidomainTissue.cpp which has code of this form in two places:

    unsigned num_elements = this->mpMesh->GetNumElements();
    std::vector<c_vector<double, SPACE_DIM> > hetero_intra_conductivities;

    c_vector<double, SPACE_DIM> intra_conductivities;
    this->mpConfig->GetIntracellularConductivities(intra_conductivities);//this one is used just for resizing

    if (this->mpConfig->GetConductivityHeterogeneitiesProvided())
    {
        try
        {
            assert(hetero_intra_conductivities.size()==0);
            hetero_intra_conductivities.resize(num_elements, intra_conductivities);
        }
    ...

The compilation error comes from a warning when applying the resize method to the std::vector:

[ 57%] Building CXX object heart/CMakeFiles/chaste_heart.dir/src/tissue/ExtendedBidomainTissue.cpp.o
In file included from /usr/include/c++/13/algorithm:60,
                 from /usr/include/boost/numeric/ublas/storage.hpp:16,
                 from /usr/include/boost/numeric/ublas/vector.hpp:21,
                 from /home/fergus/GitRepos/Chaste/Chaste/linalg/src/UblasVectorInclude.hpp:43,
                 from /home/fergus/GitRepos/Chaste/Chaste/heart/src/tissue/ExtendedBidomainTissue.cpp:36:
In static member function ‘static _Up* std::__copy_move<_IsMove, true, std::random_access_iterator_tag>::__copy_m(_Tp*, _Tp*, _Up*) [with _Tp = const double; _Up = double; bool _IsMove = false]’,
    inlined from ‘_OI std::__copy_move_a2(_II, _II, _OI) [with bool _IsMove = false; _II = const double*; _OI = double*]’ at /usr/include/c++/13/bits/stl_algobase.h:506:30,
    inlined from ‘_OI std::__copy_move_a1(_II, _II, _OI) [with bool _IsMove = false; _II = const double*; _OI = double*]’ at /usr/include/c++/13/bits/stl_algobase.h:533:42,
    inlined from ‘_OI std::__copy_move_a(_II, _II, _OI) [with bool _IsMove = false; _II = const double*; _OI = double*]’ at /usr/include/c++/13/bits/stl_algobase.h:540:31,
    inlined from ‘_OI std::copy(_II, _II, _OI) [with _II = const double*; _OI = double*]’ at /usr/include/c++/13/bits/stl_algobase.h:633:7,
    inlined from ‘boost::numeric::ublas::c_vector<T, N>& boost::numeric::ublas::c_vector<T, N>::operator=(const boost::numeric::ublas::c_vector<T, N>&) [with T = double; long unsigned int N = 1]’ at /usr/include/boost/numeric/ublas/vector.hpp:2556:21,
    inlined from ‘typename __gnu_cxx::__enable_if<(! std::__is_scalar<_Tp>::__value), void>::__type std::__fill_a1(_ForwardIterator, _ForwardIterator, const _Tp&) [with _ForwardIterator = boost::numeric::ublas::c_vector<double, 1>*; _Tp = boost::numeric::ublas::c_vector<double, 1>]’ at /usr/include/c++/13/bits/stl_algobase.h:919:11,
    inlined from ‘void std::__fill_a(_FIte, _FIte, const _Tp&) [with _FIte = boost::numeric::ublas::c_vector<double, 1>*; _Tp = boost::numeric::ublas::c_vector<double, 1>]’ at /usr/include/c++/13/bits/stl_algobase.h:977:21,
    inlined from ‘void std::fill(_ForwardIterator, _ForwardIterator, const _Tp&) [with _ForwardIterator = boost::numeric::ublas::c_vector<double, 1>*; _Tp = boost::numeric::ublas::c_vector<double, 1>]’ at /usr/include/c++/13/bits/stl_algobase.h:1007:20,
    inlined from ‘void std::vector<_Tp, _Alloc>::_M_fill_insert(iterator, size_type, const value_type&) [with _Tp = boost::numeric::ublas::c_vector<double, 1>; _Alloc = std::allocator<boost::numeric::ublas::c_vector<double, 1> >]’ at /usr/include/c++/13/bits/vector.tcc:556:14:
/usr/include/c++/13/bits/stl_algobase.h:437:30: error: ‘void* __builtin_memcpy(void*, const void*, long unsigned int)’ forming offset 24 is out of the bounds [0, 24] of object ‘__tmp’ with type ‘std::vector<boost::numeric::ublas::c_vector<double, 1>, std::allocator<boost::numeric::ublas::c_vector<double, 1> > >::_Temporary_value’ [-Werror=array-bounds=]
  437 |             __builtin_memmove(__result, __first, sizeof(_Tp) * _Num);
      |             ~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Specifically, the compiler appears to indicate a problem when copying a c_vector into multiple new spaces in a std::vector, but, only when SPACE_DIM is 1.

Even though the warning (treated as an error) is array-bounds, ignoring that specific warning does not appear to silence the problem. Possibly because the warning actually appears to be emitted from STL code that is inlined.

I have opened an issue with ublas to see if they have any thoughts: https://github.com/boostorg/ublas/issues/195

fcooper8472 commented 1 month ago

Steps to reproduce:

  1. GCC 13.x or newer
  2. Compile in release mode
  3. Current WIP on branch https://github.com/Chaste/Chaste/tree/197-new-docker-images
cmake -DCMAKE_BUILD_TYPE=Release ..
make chaste_heart -j10