Opened 13 years ago

Closed 13 years ago

#396 closed request (fixed)

Interfacing MatrixWeighted with Matrix and MatrixLookupWeighted

Reported by: Peter Owned by: Peter
Priority: major Milestone: yat 0.5
Component: utility Version: trunk
Keywords: Cc:

Description (last modified by Peter)

This include:

  1. Construct MatrixLookupWeighted from MatrixWeighted
  2. Construct MatrixWeighted from two Matrix
  3. Construct MatrixWeighted from one Matrix (and use nan function to calcaulate weights)

Having these, I think we should deprecate a number of constructors in MatrixLookupWeighted that take one or two Matrix. Also I think it is preferable to change the internal representation in MatrixLookupWeighted. Currently it is two Matrix, and I think it makes more sense to have one MatrixWeighted. There is a competition between allowing fast construction MatrixLookupWeighted(const MatrixWeighted&) and MatrixLookupWeighted(const Matrix&, const Matrix&), because when the argument is not the same as the internal representation one has to copy. I think creating from a MatrixWeighted is more important and another reason to change the internal representation to MatrixWeighted is that the iterators will be faster since we can work with references and avoid the current proxies (that have to create bot data and weight also when we are only interested in one of data or weight).

needed for ticket:444

Change History (14)

comment:1 Changed 13 years ago by Peter

Owner: changed from Jari Häkkinen to Peter
Status: newassigned

Constructors 2) and 3) were added in [1384].

comment:2 Changed 13 years ago by Peter

Changing MatrixLookupWeighted to hold a MatrixWeighted rather than two Matrix would have an important change, namely that constructor of type

MatrixLookupWeighted(const Matrix&, const Matrix&)

will copy the values (and not as before copy the two references. This implies that if any of the Matrix is changed later on (outside the MatrixLookupWeighted?) it will not modify the value in MatrixLookupWeighted?. Currently that is not the case, since MatrixLookupWeighted? holds by reference.

This opens the question: should be finish this change? If yes, how to deal with these constructors?

I see three alternatives.

  1. Keep underlying data as two Matrix as in yat 0.4
  2. Change to MatrixWeighted as described above and deprecate these constructors.
  3. Change to MatrixWeighted as described above and remove these constructors. The motivation for this argument would be that it is better to remove a functionality than keeping it and changing the behavior.

comment:3 Changed 13 years ago by Peter

No feedback from anyone, so I guess all alternatives are fine. I prefer 3) and will go with that.

comment:4 Changed 13 years ago by Peter

(In [1482]) refs #396 stupid implementation of constructors - copying should be avoided when internal representation in MatrixLookupWeighted? has changed

comment:5 Changed 13 years ago by Peter

(In [1483]) refs #396 - removing a constructor in MatrixLookupWeighted? - needed to re-organize tests in ncc_test to track down an error

comment:6 Changed 13 years ago by Peter

(In [1484]) refs #396

comment:7 Changed 13 years ago by Peter

Description: modified (diff)

comment:8 Changed 13 years ago by Peter

(In [1580]) refs #396

comment:9 Changed 13 years ago by Peter

(In [1581]) refs #396 - fixing in KernelLookup?

comment:10 Changed 13 years ago by Peter

(In [1582]) refs #396 - fixing distance_test

comment:11 Changed 13 years ago by Peter

(In [1584]) refs #396 - fixing feature_selector_test

comment:12 Changed 13 years ago by Peter

(In [1585]) refs #396 - fixing iterator_test

comment:13 Changed 13 years ago by Peter

(In [1586]) fixing knn_test - refs #396

comment:14 Changed 13 years ago by Peter

Resolution: fixed
Status: assignedclosed

(In [1587]) closes #396

Note: See TracTickets for help on using tickets.