rinuboney / clatern

Machine Learning in Clojure
Eclipse Public License 1.0
67 stars 12 forks source link

Add functionality to decision trees in preparation for adding Random Forests #36

Closed rnowling closed 9 years ago

rnowling commented 9 years ago

Changes include:

rinuboney commented 9 years ago

We should also implement predict-prob and predict-log-prob for other classifiers like logistic regression. Is clatern.protocols the right namespace to be used when a user wants to use predict-prob or predict-log-prob?

rnowling commented 9 years ago

I looked at clojure.core.matrix for an example. They provide top-level functions that call the protocols -- the users aren't directly exposed to the protocols. We could do something similar: provide clatern/predict-prob and clatern/predict-log-prob that call protocols/predict-prob and protocols/predict-log-prob. If we do that, I think we should call provide clatern/predict. What do you think?

rinuboney commented 9 years ago

yeah we could add predict-prob, predict-log-prob and predict to clatern.core

rnowling commented 9 years ago

That would work for me. I can create a new PR to add those wrapper functions and tests if that works for you.

rinuboney commented 9 years ago

That would work for me. Go ahead! There is one more thing. The function that returns the model is called decision-tree. By convention it should be the name of the training algorithm. So, isn't it supposed to be something like cart?

rnowling commented 9 years ago

You're right. I've seen CART used ambiguously. In Machine Learning: A Probabilistic Perspective by Murphy, CART is used to refer to the trees but no name is given to the training algorithm. In sklearn, CART is also used to refer to the training algorithm. If you want to adopt the sklearn nomenclature, I'd be happy to rename the function to cart.

rinuboney commented 9 years ago

Yeah I've seen CART used to refer to the trees but to go with the convention of Clatern, I think it would better to name it as cart.

rnowling commented 9 years ago

Closing in favor of #37