[ENH] Bhatthacharayya distance #4111
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4111 +/- ##
=========================================
+ Coverage 85.68% 85.7% +0.01%
=========================================
Files 390 389 -1
Lines 69727 69813 +86
=========================================
+ Hits 59744 59830 +86
Misses 9983 9983 |
AndrejaKovacic
force-pushed
the
bhatthacharayya
branch
from
57be411
to
94d5249
Compare
Orange/distance/distance.py
Outdated
| @@ -644,6 +644,36 @@ class PearsonRAbsolute(CorrelationDistance): | |||
| def fit(self, _): | |||
| return PearsonModel(True, self.axis, self.impute) | |||
|
|
|||
| def _prob_dist(a): | |||
| # Makes the vector sum to one, as to mimick probability distribution. | |||
| return a/np.sum(a) | |||
Orange/distance/distance.py
Outdated
| b = _prob_dist(b) | ||
| if sp.issparse(a): | ||
| return -np.log(np.sum(np.sqrt(a.multiply(b)))) | ||
| return -np.log(np.sum(np.sqrt(a*b))) |
AndrejaKovacic
force-pushed
the
bhatthacharayya
branch
from
94d5249
to
93b0494
Compare
| @@ -157,6 +160,9 @@ def _fix_missing(): | |||
| _fix_discrete, _fix_missing, _fix_nonbinary): | |||
| if not check(): | |||
| return None | |||
| if (METRICS[self.metric_idx][0] == 'Bhattacharyya') and _min(data.X) < 0: | |||
There was a problem hiding this comment.
It is in general a bad idea to use string literals like this. Somebody will rename it ... and there we go. I'd be happier with METRICS[self.metric_idx][1] is distance.Bhattacharyya (and, by the way, you can remove the parentheses).
On the other hand, why wouldn't the distance itself (i.e. function distance. _bhattacharyya) test the values and raise ValueError("Bhattcharyya distance requires non-negative values")? The widget already catches and shows ValueError exceptions.
As I proposed on Friday, this is better because it gives a reasonable error also to anybody that would call this distance from a script. With current code, if somebody calls Bhattacharyya with negative values, (s)he will get just a RuntimeWarning: invalid value encountered in sqrt, the result will be nan ... and this nan will propagate until an unrelated function crashes down the road. Testing and raising an exception there is better because it helps debugging.
This is open for discussion -- on Friday.
There was a problem hiding this comment.
You are right, I moved it.
AndrejaKovacic
force-pushed
the
bhatthacharayya
branch
2 times, most recently
from
7766bd9
to
0bc5bb0
Compare
AndrejaKovacic
force-pushed
the
bhatthacharayya
branch
from
0bc5bb0
to
3e6b549
Compare
| # Raise an exception for infinities, nans and negative values | ||
| check_array(a, | ||
| accept_sparse=True, accept_large_sparse=True, ensure_2d=False) | ||
| if a.min() < 0: |
There was a problem hiding this comment.
@AndrejaKovacic, would this be OK, too?
Dense arrays also have a method min, so there's no need for if. Also, I think it is better to not change the exception message raised by check_array so the caller is informed that, for instance, there are nan values in the data.
There was a problem hiding this comment.
Yes, it's more informative this way.
Description of changes
A new metric is introduced in distances widget. Bhatthacharayya is used to measure the distance between distributions. Currently, the user has to already have the data that they see as a distribution. In the future, pivot could be extended to have aggregation of all features, filtered by column values, so distributions could be generated in Orange itself.
Includes