Skip to content
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

[MRG + 1] Fix gradient boosting overflow and various other float comparison on == #7970

Merged
merged 10 commits into from Mar 7, 2017
Merged

[MRG + 1] Fix gradient boosting overflow and various other float comparison on == #7970

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
06fd1f1
reintroduced isclose() and flake8 fixes to fixes.py
chenhe95 Dec 2, 2016
01bce69
changed == 0.0 to isclose(...)
chenhe95 Dec 2, 2016
ec58cda
example changes
chenhe95 Dec 2, 2016
3e1c5d9
changed back to abs() < epsilon
chenhe95 Dec 3, 2016
74daa9b
flake8 convention on file
chenhe95 Dec 3, 2016
93eb972
reverted flake8 fixes
chenhe95 Dec 8, 2016
aef9739
reverted flake8 fixes (2)
chenhe95 Dec 8, 2016
c524142
np.finfo(np.float32).tiny instead of hard coded epsilon 1e-150
chenhe95 Dec 8, 2016
b15612c
reverted to 1e-150
chenhe95 Dec 9, 2016
0de6133
whats new modified
chenhe95 Dec 14, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions doc/whats_new.rst
View file
Open in desktop
Expand Up @@ -99,6 +99,10 @@ Bug fixes
``download_if_missing`` keyword. This was fixed in :issue:`7944` by
:user:`Ralf Gommers <rgommers>`.

- Fixed a bug in :class:`sklearn.ensemble.GradientBoostingClassifier`
and :class:`sklearn.ensemble.GradientBoostingRegressor`
where a float being compared to ``0.0`` using ``==`` caused a divide by zero
error. This was fixed in :issue:`7970` by :user:`He Chen <chenhe95>`.

- Fix a bug regarding fitting :class:`sklearn.cluster.KMeans` with a
sparse array X and initial centroids, where X's means were unnecessarily
Expand Down
9 changes: 6 additions & 3 deletions sklearn/ensemble/gradient_boosting.py
View file
Open in desktop
Expand Up @@ -511,7 +511,8 @@ def _update_terminal_region(self, tree, terminal_regions, leaf, X, y,
numerator = np.sum(sample_weight * residual)
denominator = np.sum(sample_weight * (y - residual) * (1 - y + residual))

if denominator == 0.0:
# prevents overflow and division by zero
if abs(denominator) < 1e-150:
tree.value[leaf, 0, 0] = 0.0
else:
tree.value[leaf, 0, 0] = numerator / denominator
Expand Down Expand Up @@ -577,7 +578,8 @@ def _update_terminal_region(self, tree, terminal_regions, leaf, X, y,
denominator = np.sum(sample_weight * (y - residual) *
(1.0 - y + residual))

if denominator == 0.0:
# prevents overflow and division by zero
if abs(denominator) < 1e-150:
tree.value[leaf, 0, 0] = 0.0
else:
tree.value[leaf, 0, 0] = numerator / denominator
Expand Down Expand Up @@ -634,7 +636,8 @@ def _update_terminal_region(self, tree, terminal_regions, leaf, X, y,
numerator = np.sum(y_ * sample_weight * np.exp(-y_ * pred))
denominator = np.sum(sample_weight * np.exp(-y_ * pred))

if denominator == 0.0:
# prevents overflow and division by zero
if abs(denominator) < 1e-150:
Copy link
Member

Choose a reason for hiding this comment

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

Instead of 1e-150 you could use np.finfo(np.float32).eps... There is precedent for it here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm.. I am not sure. @raghavrv do you have a strong preference for np.finfo(np.float32).eps or do you think 1e-150 is still fine? I personally prefer 1e-150 because the case where this algorithm was failing at was when denominator was around 1e-309, so I felt that 1e-150 was appropriate since it's about half of 300.

>>> np.finfo(np.float).eps
2.2204460492503131e-16
>>> np.finfo(np.double).eps
2.2204460492503131e-16

It's just that I am kind of worried that those values are too large compared to 1e-150 and I'm not sure if it will cause any rounding errors.

Copy link
Member

Choose a reason for hiding this comment

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

How about .tiny then?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

>>> np.finfo(np.float32).tiny
1.1754944e-38

I suppose this seems okay!

Copy link
Member

@raghavrv raghavrv Dec 8, 2016 •
edited

Choose a reason for hiding this comment

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

I was suggesting np.finfo(np.double).tiny, which is close to e-300 for 32 64 bit and much less for 64 bit systems...

Copy link
Member

Choose a reason for hiding this comment

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

>>> np.finfo(np.double).tiny
2.2250738585072014e-308

Copy link
Member

@raghavrv raghavrv Dec 8, 2016 •
edited

Choose a reason for hiding this comment

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

Thing is if your system is 32 bit, denominator - which is the result of np.sum - can only be as low as (np.finfo(np.float<arch>).tiny) IIUC...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I actually wasn't sure about this, because to the power of -308 seemed a bit too small as well, and it can easily overflow for not very large numerators

>>> 4/np.finfo(np.double).tiny
__main__:1: RuntimeWarning: overflow encountered in double_scalars
inf

Which was originally why I had 1e-150

tree.value[leaf, 0, 0] = 0.0
else:
tree.value[leaf, 0, 0] = numerator / denominator
Expand Down