-
Notifications
You must be signed in to change notification settings - Fork 7
Group 3: linear_regression.py #6
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
base: main
Are you sure you want to change the base?
Conversation
@@ -0,0 +1,77 @@ | |||
import numpy as np |
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.
Would benefit from a statement of what the code does.
@@ -0,0 +1,77 @@ | |||
import numpy as np | |||
|
|||
def compCostFunction(estim_y, true_y): |
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.
add a doc string to explain what the function is doing
# To be deleted later | ||
# feature_1 = np.linspace(0, 2, num=100) |
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.
Can be deleted right away
@@ -0,0 +1,77 @@ | |||
import numpy as np | |||
|
|||
def compCostFunction(estim_y, true_y): |
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.
please define argument types
X = np.random.randn(100,3) # feature matrix | ||
y = 1 + np.dot(X, [3.5, 4., -4]) # target vector | ||
|
||
m = np.shape(X)[0] # nr of samples | ||
n = np.shape(X)[1] # nr of features |
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 code should be moved down to the main part of the code
cost_history.append(cost) | ||
|
||
return W, cost_history | ||
|
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.
Please move the bulk of the code underneath a main block like this:
if __name__ == "__main__":
print(params) | ||
print(history) | ||
|
||
import matplotlib.pyplot as plt |
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.
Imports should be specified at the top of the script
def test_dimensions(x, y): | ||
# this checks whether the x and y have the same number of samples | ||
assert isinstance(x, np.ndarray), "Only works for arrays" | ||
assert isinstance(y, np.ndarray), "Only works for arrays" | ||
return x.shape[0] == y.shape[0] |
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 function does not seem to be called - can it be removed?
@@ -0,0 +1,77 @@ | |||
import numpy as np | |||
|
|||
def compCostFunction(estim_y, true_y): |
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.
Convert to snake case?
m = np.shape(X)[0] # nr of samples | ||
n = np.shape(X)[1] # nr of features | ||
|
||
def iterativeLinearRegression(X, y, alpha=0.01): |
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.
Convert to snake case?
y = 1 + np.dot(X, [3.5, 4., -4]) # target vector | ||
|
||
m = np.shape(X)[0] # nr of samples | ||
n = np.shape(X)[1] # nr of features |
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.
Change global variables to capital letters
# To be deleted later | ||
# feature_1 = np.linspace(0, 2, num=100) | ||
|
||
X = np.random.randn(100,3) # feature matrix |
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 has the same name as argument 1 of iterativeLinearRegression function
This makes iterative LR via gradient descent and returns estimated parameters and history list. | ||
""" | ||
steps=500 | ||
X = np.concatenate((np.ones((m, 1)), X), axis=1) |
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.
What does this line do? Would benefit from a comment
""" | ||
This makes iterative LR via gradient descent and returns estimated parameters and history list. | ||
""" | ||
steps=500 |
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 should be an argument to the function rather than hard-coded within it. (please)
W = W - alpha * gradient | ||
if i % 10 == 0: | ||
print(f"step: {i}\tcost: {cost}") | ||
|
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.
Add a debugging argument to the function parameters and only print when it's true
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.
Very good work! Please make the requested changes before we merge.
assert isinstance(x, np.ndarray), "Only works for arrays" | ||
assert isinstance(y, np.ndarray), "Only works for arrays" |
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.
try:
return x.shape[0] == y.shape[0]
except Exception:
raise TypeError("Only works for arrays")
No description provided.