Document HoeffdingTree
#3572
Document HoeffdingTree
#3572
Conversation
|
@rcurtin it is ready to be reviewed ? |
|
Yep, ready for review 👍 |
|
Perfect, will give it a chance tomorrow |
There was a problem hiding this comment.
Very good, similar comments to the last review of the logistic regression class the following links are not working:
Also regarding the fitness function, it would be great to have more details about them if we have many, because currently, we are only mentioning the Gini Impurity.
I am not an expert in trees, so I have no idea how many fitness functions you can have, or what we have already implemented.
The last thing is double about the precision, but apart from that great work, we can merge this really soon
| const bool batchTraining, | ||
| const double successProbability); |
There was a problem hiding this comment.
Can we make the sucessProbability as ElemType?
There was a problem hiding this comment.
I didn't do this, mostly so that when you switch to using a different ElemType, as few things change in the API as possible. (That will also make the documentation job in #3584 easier.) For something like the success probability, using a double or a float or whatever other element type really won't make any serious difference, so I think it might be easier to leave as-is.
| const arma::Row<size_t>& labels, | ||
| const size_t numClasses, | ||
| const bool batchTraining, | ||
| const double successProbability, |
| const size_t numClasses = 0); | ||
| const size_t numClasses, | ||
| const bool batchTraining, | ||
| const double successProbability, |
Yep, the invalid links will be handled in a follow-up PR. 👍
Good point. I added a sentence or two with some links in b74b8bc; let me know what you think. 👍
In this case, actually the way |
|
All is good from my side, and yes one |
![mlpack-bot[bot]](https://avatars.githubusercontent.com/in/23934?s=60&v=4){kind=small}
Here's rendered documentation: https://www.ratml.org/misc/mlpack-markdown-doc/hoeffding_tree.html
As with a lot of the previous examples, I had to make fairly intrusive changes to
Train()to match the API required by the hyperparameter tuner. Because we don't yet require C++17 (where I could usestd::optional), there are a lot of overloads. Once we do, the code can be cleaned up.The only other functionality I added was a user-facing
Reset()function to reset the tree, plus a few accessors to get properties of the trees.I also added a number of tests for everything that I implemented, as well as a test to make sure that training on floating-point data works.