Re: [Boost-users] [multi_array] operator= requires resize - why?

From: Markus Werle (numerical.simulation_at_[hidden])
Date: 2007-01-24 09:01:17

Ronald Garcia <garcia <at> cs.indiana.edu> writes:

> [...]
> multi_array is primarily designed so that the parts of the state that  
> are checked, in particular the shape and number of dimensions, will  
> not change.  Resizing functionality is special for the multi_array  
> class, and is not available for the other array types provided by the  
> library (multi_array_ref, views, and subarrays). 

which makes perfect sense to me, since the semantics are "containment"
for multi_array and "reference" for all others. I am in accordance 
with this design.

Also note that besides the defect of operator= I consider multi_array
to be a perfect lib.

> Resize was added  
> to the multi_array() long after the library was introduced to boost.   

.. because I missed the review ;-)

> The class is primarily intended to keep the same shape throughout its  
> lifetime,

Isn't this a somewhat artificial usage constraint?

> but resizing is supported primarily to enable default  
> construction of multi_arrays.

So this is water on my mills: since you allow default construction it is
intuitive beaviour that an assignment following default construction
will not fail. You require a resize between default construction and
assignment and I still do not get the advantage.

What Matthias pointed out earlier weighs even more: being forced not
to rely on compiler generated copy constructor renders multi_array 
useless for many applications I can think of.

Think again:
- AFAICS ublas assignment semantics differ from yours. 
- STL assignment semantics differ from yours (!!!!)

What stands against changing operator=?
Do we break existing code?
Do we get inferior performance?
Does a possible performace loss outweigh the pain of having
to write copy constructors and assignment operators by hand?

> reshape() is particular to multi_array and multi_array_ref.

Seems OK for me, since shape is another word for "N-D way to look at 1-D data" 

> > Also I cannot understand why the interface fails to at least support
> >
> > m2.resize(m1.extents());
>
> This looks like an oversight on my part.  I will see about supporting  
> this.  Thanks for pointing it out.

Thanks for taking this serious.

> > m2.swap(array_type(m1));
> >
> > Was the lack of swap discussed during peer review?
> 
> Is there something insufficient about using std::swap?

The extra temporary perhaps?

AFAICS the following will be performed:

template<class _Ty> inline
void swap(_Ty& _Left, _Ty& _Right)
{	// exchange values stored at _Left and _Right
_Ty _Tmp = _Left;
_Left = _Right, _Right = _Tmp;
}

Swapping base_ directly and updating the extents 
would speed up the game for large extents - or am I missing something?

> >>
> >> I'm afraid that we do not agree about the place for asserts versus
> >> exceptions.
> >
> > Who is "we"?
> 
> We = you and I.  Sorry for the confusion.
> 
> > OK, this is another battlefield. Just a comment:
> > [lenghty discussion from markus snipped]
> >
> > So instead of try-call-catch I have check-then-call which I find odd.
> > [...]
> 
> Yes, this is a use that I hadn't considered before.  Furthermore, at  
> least resizing is not something likely to be present in an inner  
> loop.  I am willing to reconsider the use of asserts in light of your  
> explanation above.

Thank you very much.

> > The root of my problem with multi_array is the (IMHO) wrong assignment
> > behaviour which I really like to see changed and I'd rather narrow  
> > down the
> > discussion to that issue.
> 
> I hope that my comments above have provided some insight into the  
> design decisions underlying the library.

Yes, but I still do not agree with the design and kindly ask you
to consider implementing swap and then operator= in terms of 
copy construction and swap, unless performance considerations 
bring us to another conclusion.

Or in order to serve both worlds, especially the performance 
paranoids: what about having an operator= in terms of copy-construction 
and swap _and_ the free function
 friend unchecked_assign(multi_array & me, multi_array const & other); ?
for all inner loops.

regards, 

Markus