New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a meta information extractor #1031

Merged
merged 6 commits into from Jun 23, 2017

Conversation

Projects
None yet
2 participants
@micyril
Member

micyril commented Jun 16, 2017

Add a tool for extracting meta information about machine learning algorithms.

micyril added some commits Jun 16, 2017

Extend HAS_METHOD_FORM
Add support for specifying the minimum number of additional arguments.
Add a tool for extracting meta information
Add a tool for extracting meta information about machine learning
algorithms.
@rcurtin

I see no problems with the actual code; I made a handful of very minor comments. Let me know when you are ready for me to merge this.

typename PredictionsType,
typename WeightsType,
bool DatasetInfo,
bool NumClasses>

This comment has been minimized.

@rcurtin

rcurtin Jun 16, 2017

Member

If I am understanding correctly, you will use this to determine regression or classification types, right? NumClasses should be true for all classification types (some changes still necessary there, would you like me to do them?) and false for all regression types.

@rcurtin

rcurtin Jun 16, 2017

Member

If I am understanding correctly, you will use this to determine regression or classification types, right? NumClasses should be true for all classification types (some changes still necessary there, would you like me to do them?) and false for all regression types.

This comment has been minimized.

@micyril

micyril Jun 19, 2017

Member

Yes, you're right again. It will be nice if you normalize API across classification algorithms.

@micyril

micyril Jun 19, 2017

Member

Yes, you're right again. It will be nice if you normalize API across classification algorithms.

Show outdated Hide outdated src/mlpack/core/cv/meta_info_extractor.hpp Outdated
/* Forms of Train with weights for classification algorithms */
using WTF4 = TrainForm<MT, PT, WT, false, true>;
using WTF5 = TrainForm<MT, PT, WT, true, true>;

This comment has been minimized.

@rcurtin

rcurtin Jun 16, 2017

Member

I like the names of these variables. :)

@rcurtin

rcurtin Jun 16, 2017

Member

I like the names of these variables. :)

This comment has been minimized.

@micyril

micyril Jun 19, 2017

Member

:)

@micyril

micyril Jun 19, 2017

Member

:)

CheckPredictionsType<DecisionTree<>, arma::Row<size_t>, arma::mat,
arma::Row<size_t>>();
CheckPredictionsType<DecisionTree<>, arma::Row<char>, arma::mat,
arma::Row<char>>();

This comment has been minimized.

@rcurtin

rcurtin Jun 16, 2017

Member

Just out of curiosity, did you try alternate types for other classes than DecisionTree<> and have a failure? I think since #290 is not done yet, there may be problems with that for some classes.

@rcurtin

rcurtin Jun 16, 2017

Member

Just out of curiosity, did you try alternate types for other classes than DecisionTree<> and have a failure? I think since #290 is not done yet, there may be problems with that for some classes.

This comment has been minimized.

@micyril

micyril Jun 19, 2017

Member

I was trying it also for HoeffdingTree<> in the line 121:
CheckPredictionsType<HoeffdingTree<>, arma::Row<size_t>, arma::imat>();
Here arma::imat is used is as the data type in the Train method.

@micyril

micyril Jun 19, 2017

Member

I was trying it also for HoeffdingTree<> in the line 121:
CheckPredictionsType<HoeffdingTree<>, arma::Row<size_t>, arma::imat>();
Here arma::imat is used is as the data type in the Train method.

* A wrapper struct for holding a Train form.
*
* @tparam MatType The type of data.
* @tparam PredictionsType The type of predictions.

This comment has been minimized.

@rcurtin

rcurtin Jun 16, 2017

Member

The assumption here is that LabelsType == PredictionsType, right? I think that's completely reasonable, I just want to make sure I am comprehending fully.

@rcurtin

rcurtin Jun 16, 2017

Member

The assumption here is that LabelsType == PredictionsType, right? I think that's completely reasonable, I just want to make sure I am comprehending fully.

This comment has been minimized.

@micyril

micyril Jun 19, 2017

Member

Yes, you're right.

@micyril

micyril Jun 19, 2017

Member

Yes, you're right.

Show outdated Hide outdated src/mlpack/tests/cv_test.cpp Outdated
@micyril

This comment has been minimized.

Show comment
Hide comment
@micyril

micyril Jun 22, 2017

Member

By the way, if you don't see any problems, feel free to merge.

Member

micyril commented Jun 22, 2017

By the way, if you don't see any problems, feel free to merge.

@rcurtin rcurtin merged commit e177287 into mlpack:master Jun 23, 2017

3 checks passed

Style Checks Build finished.
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@micyril micyril changed the title from Extracting meta information to Add a meta information extractor Aug 19, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment