Skip to content

group#5: lin reg script added#9

Open
keszybz wants to merge 4 commits into
mainfrom
linear_regression_py
Open

group#5: lin reg script added#9
keszybz wants to merge 4 commits into
mainfrom
linear_regression_py

Conversation

@keszybz
Copy link
Copy Markdown
Collaborator

@keszybz keszybz commented Feb 10, 2022

No description provided.

@keszybz keszybz changed the title lin reg script added group#5: lin reg script added Feb 10, 2022
Comment thread snippets/linear_regression.py Outdated

def compCostFunction(estim_y, true_y):
E = estim_y - true_y
C = (1 / 2 * m) * np.sum(E ** 2)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where does the variable m come from here?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, global variables are terrible. Please move all the code defining variables into a function.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... and m should be a parameter that is passed into the function.

assert isinstance(y, np.ndarray), "Only works for arrays"
return x.shape[0] == y.shape[0]

# To be deleted later
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are these comments obsolete? if yes, please remove

# To be deleted later
# feature_1 = np.linspace(0, 2, num=100)

X = np.random.randn(100,3) # feature matrix
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could the variables be named with more informative names?

Comment thread snippets/linear_regression.py Outdated

return W, cost_history

params, history = iterativeLinearRegression(X, y)
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code should be restructured so that the module can be imported and does nothing.
The test code should be under if __name__ == '__main__':.

Copy link
Copy Markdown

@lilianAweber lilianAweber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add some small comments and questions

Comment thread snippets/linear_regression.py Outdated
# feature_1 = np.linspace(0, 2, num=100)

X = np.random.randn(100,3) # feature matrix
y = 1 + np.dot(X, [3.5, 4., -4]) # target vector
Copy link
Copy Markdown
Collaborator Author

@keszybz keszybz Feb 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd write this as

y = 1 + X @ [3.5, 4., -4])  # target vector


# z = 2 + y @ feature_matrix @ feature_matrix.T

params, history = iterativeLinearRegression(X, y)
print(params)

import matplotlib.pyplot as plt
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This duplicates line above…

print(params)
print(history)

import matplotlib.pyplot as plt
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be moved to the header

Comment thread snippets/linear_regression.py Outdated
# iterate until the maximum number of steps
for i in np.arange(steps): # begin the process

y_estimated = X.dot(W)
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

X @ W !!!

Comment thread snippets/linear_regression.py Outdated
cost = compCostFunction(y_estimated, y)
# Update gradient descent
E = y_estimated - y
gradient = (1 / m) * X.T.dot(E)
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 / m * X.T @ E

@keszybz
Copy link
Copy Markdown
Collaborator Author

keszybz commented Feb 10, 2022

Looking at th e plot, I think y labels should be added:
Screenshot from 2022-02-10 13-09-42

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants