Conversation
atavory
force-pushed
the
discrete_entropy
branch
3 times, most recently
from
2a95f7e to
704d387
Compare
atavory
force-pushed
the
discrete_entropy
branch
2 times, most recently
from
a5d890e to
9e3cf4f
Compare
atavory
force-pushed
the
discrete_entropy
branch
2 times, most recently
from
6129678 to
4f4894a
Compare
atavory
force-pushed
the
discrete_entropy
branch
8 times, most recently
from
0091364 to
194d03f
Compare
atavory
force-pushed
the
discrete_entropy
branch
3 times, most recently
from
0068b91 to
900147c
Compare
|
@rschlussel Could you please review this too? Have rebased it on #13574. Man thanks! |
|
@atavory looks like there are some merge conflicts. Can you rebase with master |
shixuan-fan
left a comment
shixuan-fan
left a comment
There was a problem hiding this comment.
Skimmed till DiscreteEntropyAggregation
|
|
||
| The thresholds are defined as a sequence whose :math:`j`-th entry is the :math:`j`-th smallest threshold. | ||
|
|
||
|
|
There was a problem hiding this comment.
It is actually related to this diff - double space between the start of the new section in the rst file.
| .. math :: | ||
|
|
||
| H(x) = - \int x \log_2\left(f(x)\right) dx, | ||
| h(x) = - \int x \log_2\left(f(x)\right) dx, |
There was a problem hiding this comment.
I'm not entirely sure if this is intended.
There was a problem hiding this comment.
With discrete entropy (in this diff), it seems better to be precise and use lowercase "h" for differential entropy.
presto-main/hs_err_pid49551.log
Outdated
| # See problematic frame for where to report the bug. | ||
| # | ||
|
|
||
| --------------- T H R E A D --------------- |
There was a problem hiding this comment.
This file seems to be unintended changes?
| @Override | ||
| public long getEstimatedSize() | ||
| { | ||
| if (strategy == null) { |
There was a problem hiding this comment.
I have not read the whole PR but why do we remove this null check? strategy is set in setStrategy so this check seems legit.
| @@ -25,35 +31,200 @@ | |||
| public interface DifferentialEntropyStateStrategy | |||
There was a problem hiding this comment.
Skimmed through this class assuming they are just code moves.
| histogram.add(sample, weight); | ||
| } | ||
|
|
||
| // Tmp Ami @Override |
There was a problem hiding this comment.
nit: we could clean it up :D
atavory
force-pushed
the
discrete_entropy
branch
from
900147c to
4eb8549
Compare
|
Why is this closed 😂Is it by accident? |
|
spoke to @atavory this morning. He accidentally lost his commit while rebasing. He's gonna restore it and put it back up. |
5 participants
Add sample discrete-entropy UDF.
Together with #13574, this forms the basis for mutual information classification udfs #13163.