8000 [MRG + 3] ENH linearly scale to (-0.5, 0.5) instead of [-0.4, 0.4] by raghavrv · Pull Request #4295 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]
Skip to content

[MRG + 3] ENH linearly scale to (-0.5, 0.5) instead of [-0.4, 0.4] #4295

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
Mar 15, 2015
Merged

[MRG + 3] ENH linearly scale to (-0.5, 0.5) instead of [-0.4, 0.4] #4295

merged 2 commits into from
Mar 15, 2015

Conversation

raghavrv
Copy link
Member

Initially started off addressing #3891 (comment) by using expit (sigmoid) to map the sum_of_confidences to (0, 1)...

Currently uses linear scaling to map the sum_of_confidences to (-0.5, 0.5) before adding it to the votes. (scaling factor 0.4 --> 5 - np.finfo(sum_of_confidences.dtype).eps)

@mblondel @amueller kindly take a look!

@raghavrv
Copy link
Member Author

The used sigmoid function with the parameter 0.5:

sigmoid_0 5

sigmoid_0 5

@raghavrv raghavrv force-pushed the sigmoid_decision_fn branch from facdfa6 to 7826375 Compare February 25, 2015 16:54
@coveralls
Copy link

Coverage Status

Coverage increased (+0.0%) to 95.09% when pulling 7826375 on ragv:sigmoid_decision_fn into a926626 on scikit-learn:master.

amueller
amueller reviewed Feb 25, 2015
View reviewed changes
sklearn/multiclass.py
return votes + sum_of_confidences * \
(0.4 / max(abs(max_confidences), abs(min_confidences)))
# Map the sum_of_confidences to (0, 1) using the sigmoid function
return votes + (expit(0.5*sum_of_confidences))
Copy link
Member

Choose a reason for hiding this comment

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

Why do you need a 0.5 here?

@amueller
Copy link
Member

@mblondel the sigmoid is not <1 for large scores for numeric reasons. expit(40) == 1.0, so we'd need to use .9 * expit(scores) to make sure they are < 1.0. What do you think?

@raghavrv
Copy link
Member Author

The other option would be to use a very small value (0.001 instead of the 0.5) and stretch the sigmoid further till it can handle all sane values for sum_of_confidences ( previously scores ) :)

@amueller
Copy link
Member

well why change one magic value to another?

@raghavrv
Copy link
Member Author

True that ;)

@mblondel
Copy link
Member

or maybe np.minimum(0.99, score)?

@raghavrv
Copy link
Member Author

With both options there is a minor problem that when score_1 = 38, score_2 = 40 the output in both cases will be 0.9.

There are two more ways we could deal with it :

  • scale to [0, 1] --then--> replace all 1s by the midpoint between the 2nd largest ( say 0.98 ) and 1... So it will be always in the range [0, 1) ( practically [0, (2nd largest + 1)/2] )... i.e [-9, 0, 9] --> [0, 0.5, 1] --> [0, 0.5, 0.75]
  • scale to [0, 1] --then--> sigmoid mapping to (0, 1)

Both these methods are quite weird as @amueller pointed out :)
moreover the 2nd one is 4 times slower than simply scaling ( current approach )

Perhaps we could simply replace 0.4 by 0.5 - np.finfo(float).resolution in the current approach?

# Scale the sum_of_confidences to (-0.5, 0.5) and add it with votes
scale_factor = 0.5 - np.finfo(float).resolution
sum_of_confidences *= ( scale_factor / max(abs(max_confidences),
                                           abs(min_confidences)))
return votes + sum_of_confidences

EDIT: I am suggesting 0.5 - np.finfo(float).resolution, assuming 0.5 won't be considered a magic number since its a standard scaling factor (for symmetric scaling).

@ogrisel
Copy link
Member
ogrisel commented Mar 6, 2015

If the original 0.4 magic constant is considered too magic then I think a linear scaling to 0.5 - np.finfo(sum_of_confidences.dtype).eps is explicit enough. I don't see the benefit of introducing expit.

@raghavrv raghavrv force-pushed the sigmoid_decision_fn branch from 7826375 to 29af585 Compare March 6, 2015 12:05
@raghavrv
Copy link
Member Author
raghavrv commented Mar 6, 2015

Thanks for the comment! Done!

@raghavrv raghavrv force-pushed the sigmoid_decision_fn branch from 29af585 to 0d41b33 Compare March 6, 2015 12:10
ogrisel
ogrisel reviewed Mar 6, 2015
View reviewed changes
sklearn/multiclass.py
return votes + sum_of_confidences * \
(0.4 / max(abs(max_confidences), abs(min_confidences)))
(scaling_factor / max(abs(max_confidences), abs(min_confidences)))
Copy link
Member

Choose a reason for hiding this comment

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

I would write that as:

# Scale the sum_of_confidences to (-0.5, 0.5) and add it with votes.
# The motivation is to use confidence levels as a way to break ties in the
# votes without switching any decision made based on a difference of 1
# vote.
eps = np.finfo(sum_of_confidences.dtype).eps
max_abs_confidence = max(abs(max_confidences), abs(min_confidences)))
scale = (0.5 - eps) / max_abs_confidence
return votes + sum_of_confidences * scale

@raghavrv raghavrv force-pushed the sigmoid_decision_fn branch from 0d41b33 to d6e389c Compare March 6, 2015 13:49
@raghavrv
Copy link
Member Author
raghavrv commented Mar 6, 2015

@ogrisel done!

@raghavrv raghavrv changed the title [MRG] ENH use the sigmoid to map the sum of confidences to (0, 1) [MRG] ENH linearly scale to (-0.5, 0.5) instead of [-0.4, 0.4] Mar 6, 2015
@amueller
Copy link
Member
amueller commented Mar 6, 2015

Fine with me.

@raghavrv
Copy link
Member Author
raghavrv commented Mar 6, 2015

Should I consider that a +1 ?

@mblondel Could you also please take a look at this?

@amueller amueller changed the title [MRG] ENH linearly scale to (-0.5, 0.5) instead of [-0.4, 0.4] [MRG + 1] ENH linearly scale to (-0.5, 0.5) instead of [-0.4, 0.4] Mar 6, 2015
@raghavrv raghavrv force-pushed the sigmoid_decision_fn branch from d6e389c to 342e6e4 Compare March 6, 2015 17:11
@raghavrv
Copy link
Member Author

@ogrisel Could you take a look at this?

@ogrisel
Copy link
Member
ogrisel commented Mar 14, 2015

LGTM as well. @mblondel no objection to using linear scaling instead of a logistic sigmoid? The ranking induced by any monotonic function to the (-0.5, 0.5) range should stay the same anyway.

@mblondel
Copy link
Member

Nope, no objection.

@raghavrv raghavrv changed the title [MRG + 1] ENH linearly scale to (-0.5, 0.5) instead of [-0.4, 0.4] [MRG + 3] ENH linearly scale to (-0.5, 0.5) instead of [-0.4, 0.4] Mar 15, 2015
@ogrisel
Copy link
Member
ogrisel commented Mar 15, 2015

Alright, merging. Thanks @ragv!

ogrisel added a commit that referenced this pull request Mar 15, 2015
@ogrisel
Merge pull request #4295 from ragv/sigmoid_decision_fn
4be6214 
[MRG + 3] ENH linearly scale to (-0.5, 0.5) instead of [-0.4, 0.4]
@ogrisel ogrisel merged commit 4be6214 into scikit-learn:master Mar 15, 2015
@raghavrv raghavrv deleted the sigmoid_decision_fn branch March 18, 2015 22:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@raghavrv @coveralls @amueller @mblondel @ogrisel
0