natural icon indicating copy to clipboard operation
natural copied to clipboard

Confused: Logistic regresion classifier load function

Open itamayo opened this issue 10 years ago • 2 comments

First of all, I would like to congratulate you for the nice lib what you have developed, good job =) I am a little bit confused about the load function on the logistic Regression Classifier. It make sense if your only purpose it is to use as back up loading, but if your purpose it is for initialize some system it doesnt make sense to restore anything. I would split the logic at least on two functions, for more generic use that could be used for more purposes.

https://github.com/NaturalNode/natural/blob/master/lib/natural/classifiers/logistic_regression_classifier.js#L41-L45

Best regards

itamayo avatar Sep 16 '15 09:09 itamayo

not sure i totally follow your question, the purpose of the load function to to restore a previously saved (using Classifier.save https://github.com/NaturalNode/natural/blob/master/lib/natural/classifiers/classifier.js#L131-L140) classifier which would have been constructed using calls like addDocument.

kkoch986 avatar Sep 17 '15 14:09 kkoch986

Thanks for your answer.

/*
 * logictis_regression_classifier
*/
function load(filename, stemmer, callback) {
    Classifier.load(filename, function(err, classifier) {
            // classifier === undefined
             callback(err, restore(classifier, stemmer));
    });
}
/*
*  classifier.js
*/
function load(filename, callback) {
    var fs = require('fs');

    fs.readFile(filename, 'utf8', function(err, data) {
        var classifier;

        if(!err) {
            classifier = JSON.parse(data);
        }

        if(callback)
           // err === true so classifier === undefined
            callback(err, classifier);
    });
}

logictis_regression_classifier.load calls to classifier.load function. Although classifier.load cannot load the file, is passed back to logictis_regression_classifier.load and in this function without check the error it tries restore when classifier is undefined. So you get a error at :

/*
* classifier.js
*/

function restore(classifier, stemmer) {
   // At this point Classifier is undefined
  // if the load function it doesn't find the file required. 
   classifier.stemmer = stemmer || PorterStemmer;
    classifier.events = new events.EventEmitter();
    return classifier;
}

One solution could be : https://github.com/NaturalNode/natural/compare/master...itamayo:patch-1

itamayo avatar Sep 18 '15 11:09 itamayo