-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Added checks for relative input shapes in linear regression using the utility introduced in #2370 #3640
Added checks for relative input shapes in linear regression using the utility introduced in #2370 #3640
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -163,7 +163,6 @@ void BINDING_FUNCTION(util::Params& params, util::Timers& timer) | |
timer.Start("load_responses"); | ||
responses = params.Get<rowvec>("training_responses"); | ||
timer.Stop("load_responses"); | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, what I meant was that there was no need to replace the blank line with a line that has four spaces. I didn't mean that we should remove the line. 👍 |
||
if (responses.n_cols != regressors.n_cols) | ||
{ | ||
Log::Fatal << "The responses must have the same number of columns " | ||
|
@@ -198,16 +197,14 @@ void BINDING_FUNCTION(util::Params& params, util::Timers& timer) | |
mat points = std::move(params.Get<mat>("test")); | ||
|
||
// Ensure that test file data has the right number of features. | ||
if ((lr->Parameters().n_elem - 1) != points.n_rows) | ||
try | ||
{ | ||
util::CheckSameDimensionality(points, lr->Parameters().n_elem - 1, | ||
"Linear Regression Prediction", "test points"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure it's even necessary to keep this check--- There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sure I'll remove this check There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done @rcurtin There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the build tests failed upon removing this check I reverted this change and made a commit. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What did you try to debug the issue? "The build tests failed" is not a sufficient reason to give up on the comment here. |
||
} | ||
catch (std::invalid_argument& e) | ||
{ | ||
// If we built the model, nothing will free it so we have to... | ||
const size_t dimensions = lr->Parameters().n_elem - 1; | ||
if (computeModel) | ||
delete lr; | ||
|
||
Log::Fatal << "The model was trained on " << dimensions << "-dimensional " | ||
<< "data, but the test points in '" << testOutput << "' are " | ||
<< points.n_rows << "-dimensional!" << endl; | ||
Log::Fatal << e.what() << std::endl; | ||
} | ||
|
||
// Perform the predictions using our model. | ||
|
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 believe we can remove the
nRows
variable now; it appears to be unused.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.
Sure I'll remove the nRows declaration.
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.
done @rcurtin