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.

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

[FIX] Fix dimensionality of probabilities from values #4629

Merged
merged 2 commits into from
Apr 9, 2020

Conversation

PrimozGodec
Copy link
Contributor

@PrimozGodec PrimozGodec commented Apr 7, 2020

Issue

SGD (same than some other models) outputs wrong shapes of probabilities when those retrieved from values (when skl_learner do not provide probabilities). The issue is caused by the one_hot encoding function that does not consider a number of values in the domain.

SGD (skl_learner) for some of the losses provides probabilities that are not used by Orange since it uses custom predict function.

Description of changes
  • one_hot function changed to accept dim parameter which tells what must be a resulting number of columns
  • __call__ function for model calls one_hot with a dim parameter that has a number of values in the domain as a shape.
  • SGD model now uses a predict function from SklModel (which correctly retrieve probabilities from skl_learner) if possible.
Includes
  • Code changes
  • Tests
  • Documentation

@PrimozGodec PrimozGodec changed the title Enable probabilities for SGD, and fix dimensionality of probabilities from values [FIX] Enable probabilities for SGD, and fix dimensionality of probabilities from values Apr 7, 2020
@codecov
Copy link

codecov bot commented Apr 7, 2020

Codecov Report

Merging #4629 into master will decrease coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #4629      +/-   ##
==========================================
- Coverage   83.65%   83.63%   -0.02%     
==========================================
  Files         281      276       -5     
  Lines       56269    55457     -812     
==========================================
- Hits        47072    46384     -688     
+ Misses       9197     9073     -124     

@PrimozGodec PrimozGodec changed the title [FIX] Enable probabilities for SGD, and fix dimensionality of probabilities from values [FIX] Fix dimensionality of probabilities from values Apr 8, 2020
Copy link
Contributor

@lanzagar lanzagar left a comment

Choose a reason for hiding this comment

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

There could be one more test for one_hot which would check the case when dim is too low (since we check for that case in the code).

Also the use of one_hot in Orange/widgets/data/utils/histogram.py could be updated to use dim (removing the need for a lengthy comment and the custom fix).

Orange/data/util.py Outdated Show resolved Hide resolved
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