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
Remove nonsensical CLinearMachine::compute_bias #4381
Remove nonsensical CLinearMachine::compute_bias #4381
Conversation
karlnapf
commented
Jul 20, 2018
- add bias coputation to CLinearRidgeRegression
- activate bias in LibLinear and LibLinearRegression by default
- cleanups
1b3a8bd
to
4643b82
Compare
* add bias coputation to CLinearRidgeRegression * activate bias in LibLinear and LibLinearRegression by default * cleanups
49a111b
to
ab88ee7
Compare
ab88ee7
to
8862afd
Compare
@@ -88,6 +88,10 @@ template<class ST> CFeatures* CDenseFeatures<ST>::duplicate() const | |||
return new CDenseFeatures<ST>(*this); | |||
} | |||
|
|||
{ | |||
return result; | |||
} |
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.
This must have slipped through.
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.
y
@@ -1,5 +1,6 @@ | |||
#include <shogun/labels/DenseLabels.h> | |||
#include <shogun/labels/RegressionLabels.h> | |||
#include <shogun/mathematics/linalg/LinalgNamespace.h> |
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.
Not necessary or was implicit include?
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.
unnecessary
@@ -71,11 +71,14 @@ bool CLinearRidgeRegression::train_machine(CFeatures* data) | |||
linalg::matrix_prod(feats_matrix, feats_matrix, kernel_matrix, false, true); | |||
linalg::add_diag(kernel_matrix, tau_vector); | |||
|
|||
auto labels = ((CRegressionLabels*)m_labels)->get_labels(); |
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.
Minor: fix indentation of this block to rest of the function?
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.
will do autoformat once this works, thx
@iglesias actually the bias in the previous version was still wrong, I double checked things. Now it should work. |
Travis times out, but all tests pass locally, merging |
@karlnapf all right. I did not check the implementation of the equations 8-D |
* add/correct bias computation to CLinearRidgeRegression * cleanups