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 accuracy and mean squared error #1016
Conversation
const DataType& data, | ||
arma::mat& responses) | ||
{ | ||
// In the case of neural networks data should be passed without const |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 400abaa, so I think we can remove the extra function here.
Simplify the MSE implementation after merging with the master branch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. If you can handle the CMake comment I'll go ahead and merge.
# Define the files we need to compile | ||
# Anything not in this list will not be compiled into mlpack. | ||
set(SOURCES | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Either we should add the metrics/*
files here, or an add_subdirectory(metrics)
call somewhere in this file so that the new files are properly added to MLPACK_SOURCES
. Let me know if I can clarify anything about that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have too much experience with cmake, and I was looking at src/mlpack/core/data/CMakeLists.txt
as an example while writing src/mlpack/core/cv/CMakeLists.txt
, since src/mlpack/core/data
also has some directories. Probably we should fix src/mlpack/core/data/CMakeLists.txt
too. I will fix src/mlpack/core/cv/CMakeLists.txt
shortly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, nice catch, I will commit changes for data/
shortly.
Add the metrics accuracy and mean squared error for cross-validation.