-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Implemention of Mean Method in distributions.Categorical #1411
base: main
Are you sure you want to change the base?
Conversation
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here with What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
1 similar comment
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here with What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
@googlebot I signed it! |
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
1 similar comment
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
This implementation is not correct. The mean of a categorical is defined as This will also need tests to verify the implementation is correct (entropy tests, again, should provide a good set of test cases). |
Hello @SiegeLordEx, Thanks for your response.
Check this out |
The previous implementation was incorrect. I've written an example implementation of how the Categorical mean should be, the code below works for me and produces the desired result. The mean should also account for unnormalised probabilities between the classes, since there are cases where the probabilities might not sum up to 1. def _mean(self):
probs = self.probs_parameter()
num_categories = self._num_categories(probs)
# Initialise the mean with zeros
categorical_mean = tf.zeros(tf.shape(probs[...,0]))
# Compute the normalisation factors such that all probabilities sum up to 1
normalisation = tf.reduce_sum(probs, axis=-1)
# Sum up all the normalised probabilities
# sum(i * prob(X=i) for i in range(num_categories) )
for i in range(num_categories):
categorical_mean = categorical_mean + tf.cast(i,probs.dtype) * probs[...,i] / normalisation
return categorical_mean Shall I make a new commit/pull request with this? |
I don't think there is currently an implementation.
The following might be simpler:
tf.reduce_sum(tf.range(self._num_categories(probs)) * probs, axis=-1) /
tf.reduce_sum(probs, axis=-1)
You could send a PR, sure.
…On Fri, May 5, 2023 at 9:58 AM Fotis Drakopoulos ***@***.***> wrote:
The previous implementation was incorrect. I've written an example
implementation of how the Categorical mean should be like, the code below
works for me and produces the desired result. The mean should also account
for unnormalised probabilities between the classes, since there are cases
where the probabilities don't sum up to 1.
def _mean(self):
probs = self.probs_parameter()
num_categories = self._num_categories(probs)
# Initialise the mean with zeros
categorical_mean = tf.zeros(tf.shape(probs[...,0]))
# Compute the normalisation factors such that all probabilities sum up to 1
normalisation = tf.reduce_sum(probs, axis=-1)
# Sum up all the normalised probabilities
# sum(i * prob(X=i) for i in range(num_categories) )
for i in range(num_categories):
categorical_mean = categorical_mean + tf.cast(i,probs.dtype) * probs[...,i] / normalisation
return categorical_mean
Shall I make a new commit for this?
—
Reply to this email directly, view it on GitHub
<#1411 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AFJFSI3YPLRHUW55BLBNKCDXEUBRPANCNFSM5CVAO7SQ>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
Hi, Indeed this looks much simpler, thanks! We want to ensure that the multiplication is applied across the last axis of probs, i.e. each slice of probs across the last dimension (probs[...,i]) gets multiplied by each number i in tf.range(self._num_categories(probs)). The multiplication should be doing this by default, but I will test the code and submit a new PR. |
Proposition for correcting issue #685 Error with Implementation of Mean Method in distributions.Categorical
Defined a
_mean
method for implementing mean