Subject: Re: [ublas] An alternative approach to shallow_array_adaptor using storage class
From: Guillermo Ruiz Troyano (ruiztroyano_at_[hidden])
Date: 2012-03-01 04:38:32
Thanks, but this class throw an exception when you try to assign an expression to the vector:
using namespace boost::numeric::ublas;
struct point { double x; double y; double z; };
point p = { 1, 2, 3 };
vector<double,my_unbounded_array<double> > v(my_unbounded_array<double>(3, .0));
v.data().bind(3, &p.x);
v = v*2.0; // <- terminate called throwing an exception!
ublas::vector calls to swap of storage class when it needs to assign a temporary to self (case of assign an expression). When own_==false swap function should swap elements of the array, but your function throw an exception. See swap function of bounded_array for example.
If both own_==true then swap pointer and size (like unbounded_array)
If one own_==false then swap elements of array (like bounded_array)
> // Swapping
> BOOST_UBLAS_INLINE
> void swap (my_unbounded_array& a) {
> if (own_ && a.own_) {
> if (this != &a) {
> std::swap(size_, a.size_);
> std::swap(own_, a.own_);
> std::swap(data_, a.data_);
> }
> }
> else if (data_ != a.data_)
> std::swap_ranges(data_.get(), data_.get()+size_, a.data_.get());
> }
Regards,
Guillermo Ruiz Troyano.
El 28/02/2012, a las 18:46, Savyasachi Singh escribió:
> I had the need for a functionality similar to shallow_array_adaptor. I wrote a quick and dirty solution based on ublas::unbounded_array. I call this storage class my_unbounded_array which can hold data with/without ownership. The implementation is as follows
>
> // Storage class
> template <class T>
> class my_unbounded_array : public boost::numeric::ublas::storage_array<my_unbounded_array<T>> {
> private:
> typedef my_unbounded_array<T> self_type;
> public:
> typedef std::size_t size_type;
> typedef std::ptrdiff_t difference_type;
> typedef T value_type;
> typedef const T& const_reference;
> typedef T& reference;
> typedef const T* const_pointer;
> typedef T* pointer;
> typedef const_pointer const_iterator;
> typedef pointer iterator;
> typedef std::reverse_iterator<const_iterator> const_reverse_iterator;
> typedef std::reverse_iterator<iterator> reverse_iterator;
>
>
> inline explicit my_unbounded_array() : own_(true), size_(0), data_(nullptr) {};
>
> inline explicit my_unbounded_array(size_type n) : own_(true), size_(n), data_(allocate(n)) {}
>
> // If bind is true then just copy the address of external data
> inline my_unbounded_array(size_type n, pointer arr, bool bind=false) : own_(!bind), size_(n), data_(nullptr) {
> if(bind) data_.reset(arr);
> else {
> data_.reset(allocate(n));
> std::uninitialized_copy(arr, arr+n, begin());
> }
> }
>
> inline my_unbounded_array(size_type n, const value_type& init) : own_(true), size_(n), data_(allocate(n)) {
> std::uninitialized_fill_n(begin(), n, init);
> }
>
> inline my_unbounded_array(const my_unbounded_array& rhs) : own_(true), size_(rhs.size_), data_(allocate(rhs.size_)) {
> std::uninitialized_copy(rhs.begin(), rhs.end(), begin());
> }
>
> inline ~my_unbounded_array() {
> if(!own_) data_.release();
> }
>
> inline size_type size() const {return size_;}
>
> inline bool owner() const {return own_;}
>
> inline bool empty() const {return size_==0;}
>
> //! Bind to external data. Set ownership to false.
> inline void bind(size_type n, pointer arr) {
> if(!own_) data_.release();
> own_=false;
> size_=n;
> data_.reset(arr);
> }
>
> inline void resize(size_type n) {
> if(size_!=n) {
> if(!own_) {
> data_.release();
> throw("Cannot resize external data."); // Throw an appropriate exception here. This is just an example
> }
> data_.reset(allocate(n));
> size_=n;
> }
> }
>
> inline void resize(size_type n, value_type init) {
> if(size_!=n) {
> if(!own_) {
> data_.release();
> throw("Cannot resize external data."); // Throw an appropriate exception here. This is just an example
> }
> pointer tmp=nullptr;
> if(n) {
> tmp=allocate(n);
> if(size_<n) {
> std::uninitialized_copy(begin(), end(), tmp);
> std::uninitialized_fill(tmp+size_, tmp+n, init);
> } else
> std::uninitialized_copy(begin(), begin()+n, tmp);
> }
> size_=n;
> data_.reset(tmp);
> }
> }
>
>
> // Element access
> inline const_reference operator[](size_type i) const {
> return (data_.get())[i];
> }
>
> inline reference operator[](size_type i) {
> return (data_.get())[i];
> }
>
> // Assignment
> inline my_unbounded_array& operator=(const my_unbounded_array& rhs) {
> if(this!=&rhs) {
> resize(rhs.size_);
> std::copy(rhs.data_.get(), (rhs.data_.get())+rhs.size_, data_.get());
> }
> return *this;
> }
>
> inline my_unbounded_array& assign_temporary(my_unbounded_array& rhs) {
> swap(rhs);
> return *this;
> }
>
> inline void swap(my_unbounded_array& rhs) {
> if(!own_) {
> data_.release();
> throw("Cannot resize external data."); // Throw an appropriate exception here. This is just an example
> }
> if(this!=&rhs) {
> std::swap(size_, rhs.size_);
> std::swap(own_, rhs.own_);
> data_.swap(rhs.data_);
> }
> }
>
> friend inline void swap(my_unbounded_array& x, my_unbounded_array& y) {x.swap(y);}
>
> inline const_iterator begin() const {return data_.get();}
>
> inline const_iterator end() const {return data_.get()+size_;}
>
> inline iterator begin() {return data_.get();}
>
> inline iterator end() {return data_.get()+size_;}
>
> inline const_reverse_iterator rbegin() const {return const_reverse_iterator(end());}
>
> inline const_reverse_iterator rend() const {return const_reverse_iterator(begin());}
>
> inline reverse_iterator rbegin() {return const_reverse_iterator(end());}
>
> inline reverse_iterator rend() {return const_reverse_iterator(begin());}
>
> private:
> friend class boost::serialization::access;
>
> // Serialization
> template<class Archive>
> void serialize(Archive& ar, const unsigned int version) {
> boost::serialization::collection_size_type s(size_);
> ar& boost::serialization::make_nvp("size", s);
> if(Archive::is_loading::value) resize(s);
> ar& boost::serialization::make_array(data_.get(), s);
> }
>
> static inline pointer allocate(const size_t n) {
> try {
> return n>0 ? new T[n] : nullptr;
> } catch(std::bad_alloc& e) {
> // Do something here!!!!!!!!!!!!
> }
> }
>
> //! Data ownership. True if it owns the memory block.
> /*!If false then data is treated as external and not deleted by the destructor,
> * and any operation involving resizing is not allowed. */
> bool own_;
> size_type size_;
> std::unique_ptr<T> data_;
> };
> // End of class my_unbounded_array
>
> // Usage example
> // vector<double, my_unbounded_array<double>> x(10, 0.); // x is just like normal ublas vector
> // x.bind(10, ptr); // x is now holding external data pointed by raw pointer (ptr)
>
> I hope this helps.
>
> Sincerely,
> Savyasachi Singh
> Univ. of Florida
>
>
>
> On Tue, Feb 28, 2012 at 12:00 PM, <ublas-request_at_[hidden]> wrote:
> Send ublas mailing list submissions to
> ublas_at_[hidden]
>
> To subscribe or unsubscribe via the World Wide Web, visit
> http://listarchives.boost.org/mailman/listinfo.cgi/ublas
> or, via email, send a message with subject or body 'help' to
> ublas-request_at_[hidden]
>
> You can reach the person managing the list at
> ublas-owner_at_[hidden]
>
> When replying, please edit your Subject line so it is more specific
> than "Re: Contents of ublas digest..."
>
> Today's Topics:
>
> 1. shallow_array_adaptor bug and fixes (Guillermo Ruiz Troyano)
>
>
> ---------- Forwarded message ----------
> From: Guillermo Ruiz Troyano <ruiztroyano_at_[hidden]>
> To: ublas_at_[hidden]
> Cc:
> Date: Tue, 28 Feb 2012 14:10:41 +0100
> Subject: [ublas] shallow_array_adaptor bug and fixes
> I'm been trying to use shallow_array_adaptor as storage with ublas::vector to modify external data. When you try this:
>
> #define BOOST_UBLAS_SHALLOW_ARRAY_ADAPTOR
> #include <boost/numeric/ublas/vector.hpp>
>
> struct point {
> double x;
> double y;
> double z;
> };
>
> void test() {
> using namespace boost::numeric::ublas;
>
> point p = { 1, 2, 3 }
> shallow_array_adaptor<double> a(3, &p.x); // Ok, a holds p address
> vector<double, shallow_array_adaptor<double> > v(a); // Ok, v holds p address
>
> v += v; // Ok, now p = { 2, 4, 6 }
> v /= 2; // Ok, now p = { 1, 2, 3 }
> v = v*2; // <- Oh no, p = { 1, 2, 3 } !!!
> }
>
> The bug comes from swap function of shallow_array_adapter that is used by vector. When you assign v with an expression, this constructs a temporary and then swaps with it. The issue is that it swaps pointers without check if both objects own the data:
>
> class shallow_array_adaptor<T> : … {
> ...
> // Swapping
> BOOST_UBLAS_INLINE
> void swap (shallow_array_adaptor &a) {
> if (this != &a) {
> std::swap (size_, a.size_);
> std::swap (own_, a.own_);
> std::swap (data_, a.data_); // This is definitely infidelity
> }
> }
> ...
> };
>
> When both are data owners can do that (as unbounded_array does). But if they are not then it should swap as bounded_array does. Something like this:
>
> class shallow_array_adaptor<T> : … {
> ...
> // Swapping
> BOOST_UBLAS_INLINE
> void swap (array_adaptor& a) {
> if (own_ && a.own_) {
> if (this != &a) {
> std::swap(size_, a.size_);
> std::swap(own_, a.own_);
> std::swap(data_, a.data_);
> }
> }
> else if (&data_[0] != &a.data_[0])
> std::swap_ranges(data_, data_+size_, a.data_);
> }
> ...
> };
>
> Fixing this function I get:
>
> void test() {
> point p = { 1, 2, 3 }
> shallow_array_adaptor<double> a(3, &p.x); // Ok, a holds p address
> vector<double, shallow_array_adaptor<double> > v(a); // Ok, v holds p address
> v = v*2; // <- Ok, now p = { 2, 4, 6 } and v holds p address!
> }
>
> Sometimes you need something like shallow_array_adaptor. On the other hand, array_adaptor is a redundant class that unbounded_array or bounded_array can do if they have a constructor like this (or with a pair of iterators as parameters):
>
> unbounded_array(size_t size, T* data)
> : size_(size), data_(new value_type[size]) {
> std::copy(data, data+size, data_);
> }
>
> I would like to see:
> 1. A fixed shallow_array_adaptor or a similar storage class that references external data.
> 2. unbounded_array and bounded_array constructor that you pass an iterator range for copy. adaptor_array is to use a sledgehammer to crack a nut!
>
> Regards,
> Guillermo Ruiz Troyano
>
> _______________________________________________
> ublas mailing list
> ublas_at_[hidden]
> http://listarchives.boost.org/mailman/listinfo.cgi/ublas
>
>
> _______________________________________________
> ublas mailing list
> ublas_at_[hidden]
> http://listarchives.boost.org/mailman/listinfo.cgi/ublas
> Sent to: ruiztroyano_at_[hidden]