-
-
Notifications
You must be signed in to change notification settings - Fork 44.3k
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
Modified Linear Regression to work on OLS, fixes #8847 #11311
base: master
Are you sure you want to change the base?
Conversation
for more information, see https://pre-commit.ci
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.
Nice!
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.
Good stuff! Apart from the changes that I've requested in my other comments, could you also rewrite the explanation at the top of the file to reflect your new implementation? In particular, make sure your new explanation does the following:
- Briefly explain what linear regression is
- Explain the OLS regression formula,
$(\mathbf{X}^{\top} \mathbf{X})^{-1} \mathbf{X} \mathbf{y}$ - Note possible issues with your implementation (i.e. inefficiency, numerical instability, etc.)
- Cite a source that readers can refer to for further info (Wikipedia or similar is fine)
I wrote a similar explanation for weighted regression in machine_learning/local_weighted_learning/local_weighted_learning.py
a while back. If you'd like, you're welcome to use that explanation as a reference (though your explanation doesn't need to be nearly as long).
:param data_x : contains our dataset | ||
:param data_y : contains the output (result vector) | ||
def run_linear_regression_ols(data_x, data_y): | ||
"""Implement Linear regression using OLS over the dataset |
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.
"""Implement Linear regression using OLS over the dataset | |
"""Implement OLS linear regression over a given dataset |
Slight rewording for clarity
:return : feature for line of best fit (Feature vector) | ||
""" |
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.
The OLS regression function needs doctests—make sure you verify the outputs of your tests with a calculator that can do linear regression (e.g., Wolfram Alpha)
Co-authored-by: Tianyi Zheng <tianyizheng02@gmail.com>
Co-authored-by: Tianyi Zheng <tianyizheng02@gmail.com>
Describe your change:
Checklist: