Opened 13 years ago

Closed 13 years ago

#256 closed request (fixed)

vector matrix view design - could it be improved?

Reported by: Peter Owned by: Peter
Priority: critical Milestone: yat 0.4
Component: utility Version: trunk
Keywords: Cc:

Description (last modified by Peter)

related to ticket:267

I don't like the interface to views.

For example right now I wanna add up certain rows in a matrix. Then I have to write something like

utility::vector view(m, i);
sum += view;

I think it would be much more intuitive to have a function in matrix, so one could write

sum += m.row_view(i);

Even better is that if we define the functions as

utility::vector matrix::row_view(size_t i);
const utility::vector matrix::row_view(size_t i) const;

we have solved the problem that we can create mutable views from a const matrix.

What is the problem then? The biggest problem is that it is not possible to something like this, because the returned vector will not be a view. The vector inside the function may be a view, but once copied the copy constructor creates a pure vector, and the returned vector is not a view.

I remember long discussion preceded that decision and therefore I put this as a discussion ticket in later.

Change History (14)

comment:1 Changed 13 years ago by Peter

I'll implement a suggestion in my branch (source:peter-dev). If you like it we can merge it into trunk.

comment:2 Changed 13 years ago by Jari Häkkinen

You do not need to write

utility::vector view(m, i);
sum += view;

try

sum += utility::vector(m, i);

This is almost as short had as your preferred syntax. Anyhow, I don't understand your gibberish about some problem in the end of the description.

comment:3 Changed 13 years ago by Peter

Thanks for your tip.

Well, I will try to explain. As I see the vector (and matrix) class is that it is almost like three classes in one. Let us call it that we have different states of a vector: vector, vector_view, and vector_const_view. As of a decison taken last winter, the copy constructor does not copy this state, i.e., the constructed vector is always in state vector. We had some rationale behind that (although I cant remember it). So far it should be clear, I hope.

Ok, so this implies when we are passing by value (or returning by value) the copy constructor is called, and the constructed vector is in state vector. So even if we construct a vector with state ~vector_view` inside a function, when we return by value the returned vector will be an independent deep copy.

My idea was that when you want a vector_view into a matrix you should call the matrix. I think that is more intuitive. My point is that when we want an element_view, we have double& operator()(size_t i). We would never in any design in any language put that functionality in a double constructor. In analog, I would like to have the ability to create a vector_view from matrix interface.

I therefore thought about refactoring the vector class, but since you have announced a redesign of matrix class on your side, I have put those plans on hold.

comment:4 Changed 13 years ago by Jari Häkkinen

I agree with would never call a double constructor. The analogue to a vector view into a matrix would be vector& operator()(size_t i) but that was rejected and something was said that row and column views should be 1xN or Nx1 matrix views. If I remember our previous discussions correctly I think that the flaws you are bringing up is due to design decisions. Well, it does not make it right. Let us see if MTL4 resolves our issues. I am going to Korea for a week and the track record from my previous journeys are good for code production.

comment:5 Changed 13 years ago by Peter

I know I'm criticizing thoroughly discussed decisions here. That is why I'm so defensive (!!!).

I am not arguing for introducing column and row vectors. "A vector is a vector".

comment:6 Changed 13 years ago by Peter

Description: modified (diff)

ticket:267 has been marked as related to this ticket

comment:7 Changed 13 years ago by Peter

ticket:298 was marked as duplicate of this ticket

comment:8 Changed 13 years ago by Peter

Milestone: later0.4
Owner: changed from Jari Häkkinen to Peter
Priority: majorcritical
Status: newassigned
Type: discussionrequest

comment:9 Changed 13 years ago by Peter

implementation committed in r1009

Four things to do before closing this ticket:

  • Documentation needs to be updated
  • Change name vectorBase to VectorBase?
  • Change name vectorView to VectorView?
  • Use base class VectorBase?& rather than vector where applicable

What do you think, should we change vector to Vector or stay with vector?

comment:10 Changed 13 years ago by Peter

I forgot the key thing. VectorView? constructors from const& should be private and instead corresponding functions should be public returning const VectorView

comment:11 Changed 13 years ago by Peter

vectorView and vectorBase were changed to VectorView? and Vectorbase, respectively, in r1015

comment:13 Changed 13 years ago by Peter

Split back to VectorView? and VectorConstView? in r1026

comment:14 Changed 13 years ago by Peter

Resolution: fixed
Status: assignedclosed
Note: See TracTickets for help on using tickets.