Opened 13 years ago

Closed 13 years ago

#318 closed request (fixed)

SupervisedClassifier::train take data and target

Reported by: Peter Owned by: Markus Ringnér
Priority: major Milestone: yat 0.4
Component: classifier Version:
Keywords: Cc:

Description

daughter of ticket:287

related to ticket:259

Constructors should not take target and data, but instead we have two train functions: one for MatrixLookup? and one for MatrixLookupWeighted?.

This implies that the make_classifier function takes no arguments and can be implemented as a copy constructor.

Change History (8)

comment:1 Changed 13 years ago by Markus Ringnér

Status: newassigned

comment:2 Changed 13 years ago by Markus Ringnér

Type: defectrequest

comment:3 Changed 13 years ago by Markus Ringnér

In [1157] this has been mostly fixed.

Left-overs:

  • make_classifier in KNN/NBC/NCC should be checked, now they are potentially too minimal with regards to copying values for class members. Peter, please fix NBC, I will check the other two.
  • In KNN pointers to data and targets received in training are stored and used when predicting. Should KNN keep copies of training data and targets?
  • Peter, you mentioned something about reintroducing some tests for EnsembleBuilder? that were removed while SVM and the others were at different development levels. Can this be done now? If so can you fix it.

comment:4 in reply to:  3 ; Changed 13 years ago by Peter

Replying to markus:

In [1157] this has been mostly fixed.

Left-overs:

  • make_classifier in KNN/NBC/NCC should be checked, now they are potentially too minimal with regards to copying values for class members. Peter, please fix NBC, I will check the other two.

Sure.

  • In KNN pointers to data and targets received in training are stored and used when predicting. Should KNN keep copies of training data and targets?

The data is extremely cheap to copy - basically 3 pointers, but I think in principle that classifier should not copy these things.

  • Peter, you mentioned something about reintroducing some tests for EnsembleBuilder? that were removed while SVM and the others were at different development levels. Can this be done now? If so can you fix it.

I will look into that.

comment:5 Changed 13 years ago by Peter

(In [1161]) testing to build ensemble of svm refs #318

comment:6 in reply to:  4 Changed 13 years ago by Peter

Replying to peter:

Replying to markus:

In [1157] this has been mostly fixed.

Left-overs:

  • make_classifier in KNN/NBC/NCC should be checked, now they are potentially too minimal with regards to copying values for class members. Peter, please fix NBC, I will check the other two.

Sure.

NBC::make_classifier is fine.

comment:7 Changed 13 years ago by Markus Ringnér

make_classifier fixed for NCC and KNN too in [1164].

I guess we can close ticket if we agree that:

In the description of this ticket it is written:

"This implies that the make_classifier function takes no arguments and can be implemented as a copy constructor"

I have not implemented make_classifier as a call to the copy constructors (only default versions exist) since I think make_classifier should produce a copy but an untrained copy. Agree?

comment:8 Changed 13 years ago by Peter

Resolution: fixed
Status: assignedclosed

Agree

Note: See TracTickets for help on using tickets.