Skip to content

replace log with log1p#11424

Merged
jnothman merged 2 commits intoscikit-learn:masterfrom
meetnaren:patch-1
Jul 4, 2018
Merged

replace log with log1p#11424
jnothman merged 2 commits intoscikit-learn:masterfrom
meetnaren:patch-1

Conversation

@meetnaren
Copy link
Copy Markdown
Contributor

@meetnaren meetnaren commented Jul 4, 2018

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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Member

agramfort commented Jul 4, 2018

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

@meetnaren
Copy link
Copy Markdown
Contributor Author

meetnaren commented Jul 4, 2018

@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
Copy Markdown
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