8000 replace log with log1p by meetnaren · Pull Request #11424 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]
Skip to content

replace log with log1p #11424

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Jul 4, 2018
Merged

replace log with log1p #11424

merged 2 commits into from
Jul 4, 2018

Conversation

meetnaren
Copy link
Contributor

log1p is a more accurate implementation of log(1+x). Refer doc here:
https://docs.scipy.org/doc/numpy-1.13.0/reference/generated/numpy.log1p.html

Reference Issues/PRs

What does this implement/fix? Explain your changes.

Any other comments?

@meetnaren
replace log with log1p
007444e 
log1p is a more accurate implementation of log(1+x). Refer doc here:
https://docs.scipy.org/doc/numpy-1.13.0/reference/generated/numpy.log1p.html
jnothman
jnothman previously approved these changes Jul 4, 2018
View reviewed changes
Copy link
Member
@jnothman jnothman left a comment

Choose a reason for hiding this comment

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

LGTM if all CI pass.

@jnothman jnothman dismissed their stale review July 4, 2018 03:40

See comments

@jnothman
Copy link
Member
jnothman commented Jul 4, 2018

It would be good to fix all such cases:

git grep -n 'log.*1 +' manually filtered:

doc/modules/model_evaluation.rst:179:    ...     return np.log(1 + diff)
examples/linear_model/plot_sgd_loss_functions.py:32:plt.plot(xx, np.log2(1 + np.exp(-xx)), color='cornflowerblue', lw=lw,
examples/plot_isotonic_regression.py:31:y = rs.randint(-50, 50, size=(n,)) + 50. * np.log(1 + np.arange(n))
sklearn/calibration.py:458:    AB0 = np.array([0., log((prior0 + 1.) / (prior1 + 1.))])
sklearn/gaussian_process/gpc.py:412:                - np.log(1 + np.exp(-(self.y_train_ * 2 - 1) * f)).sum() \
sklearn/utils/_logistic_sigmoid.pyx:16:        return -log(1 + exp(-x))
sklearn/utils/_logistic_sigmoid.pyx:18:        return x - log(1 + exp(x))

git grep -n 'log.*+ 1' manually filtered:

sklearn/metrics/regression.py:317:    return mean_squared_error(np.log(y_true + 1), np.log(y_pred + 1),
sklearn/neighbors/binary_tree.pxi:489:        factor = logVn(d) - log(d + 1.)

@agramfort
Copy link
Member

@meetnaren do you want to take care of all the others or shall we merge as is?

@meetnaren
Copy link
Contributor Author

@agramfort @jnothman I've created a separate pull request for the other changes (#11428).

@jnothman jnothman merged commit 6f9a8df into scikit-learn:master Jul 4, 2018
@jnothman
Copy link
Member
jnothman commented Jul 4, 2018

Thanks

@qinhanmin2014 qinhanmin2014 mentioned this pull request Jul 5, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jnothman jnothman jnothman left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@meetnaren @jnothman @agramfort
0