Skip to content

added a new file with the vectorized version of logistic model#4

Open
Leo-Laurent wants to merge 3 commits intomasterfrom
leo_new_logistic_model
Open

added a new file with the vectorized version of logistic model#4
Leo-Laurent wants to merge 3 commits intomasterfrom
leo_new_logistic_model

Conversation

@Leo-Laurent
Copy link
Copy Markdown
Contributor

No description provided.

@Leo-Laurent Leo-Laurent requested review from amyreo and zermelozf June 11, 2020 22:17
@Leo-Laurent Leo-Laurent self-assigned this Jun 11, 2020
@Leo-Laurent Leo-Laurent reopened this Jun 11, 2020
Copy link
Copy Markdown
Member

@zermelozf zermelozf left a comment

Choose a reason for hiding this comment

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

Overall pretty good.

Spaces around operators should be reviewed a little bit.

Also, one minor improvement could be to extract the history of loss and values outside of the gradient function for more flexibility. You can have a look at what's done there: https://github.com/zermelozf/notebooks/blob/master/First%20order%20optimization.ipynb



def probability_y(x, y, theta): # array of P( Y=y[k] | x[k] )
return 1 / (1 + np.exp(-y*x.dot(theta)))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please add spaces between the * and the terms.



def generate_y(x, theta):
return np.sign(2/(1 + np.exp(-x.dot(theta))) - 1)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please add spaces around the /

def generate_real_x(n, theta):
dict_frequencies = word_frequencies_dictionary()
n = min(n, len(dict_frequencies))
x = np.array([[0., 1.]]*n)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

spaces



def logistic_error(x, y, theta):
return - np.sum(np.log(probability_y(x, y, theta)))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

no need for space here after -

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.

2 participants