cleanup of rate_distortion #25
Conversation
src/ultk/effcomm/rate_distortion.py
Outdated
|
|
||
| `distortion`: the distortion E[DKL[ M || M_hat ]] = I[M:U] - I[W:U], in bits | ||
| """ | ||
| args = [language, prior, meaning_dists] |
There was a problem hiding this comment.
incredibly minor, but best for this to be a tuple not a list
src/ultk/effcomm/rate_distortion.py
Outdated
| """ | ||
| args = [language, prior, meaning_dists] | ||
| return ib_encoder_to_point( | ||
| *args[1:], language_to_ib_encoder_decoder(*args).values() |
There was a problem hiding this comment.
Does the language_to... not also need a * in front of it?
There was a problem hiding this comment.
Actually, I feel like this might be easier to read if you just get the raw dict from language_to_ib.... in the body, and then send in in ["encoder"] and ["decoder"] here
| Returns: | ||
| array of shape `(|words|, |meanings|)` representing the 'optimal' deterministic decoder | ||
| """ | ||
| return bayes(encoder, prior) @ meaning_dists |
|
|
||
| complexity = information_cond(prior, encoder) | ||
| accuracy = MI(meaning_dists @ joint(encoder, prior)) | ||
| distortion = MI(joint(meaning_dists, prior)) - accuracy |
There was a problem hiding this comment.
a comment or two on these lines, or a bit more detail in the docstring, would be helpful
| betas, | ||
| *args, | ||
| **kwargs, | ||
| ).get_results() |
There was a problem hiding this comment.
Very cool! Wondering if we might want to expose the raw optimizer separate from the results at some place, but that's easy to do in the future if we need to
There was a problem hiding this comment.
Interested to hear more! In principle I can imagine one might want the IBOptimizer instance to look at in addition to getting a curve, since it does indeed contain a lot of useful data and methods.
Thanks for reviewing! This rdot via ultk stuff is actively being used by my signaling + IB project, which has yet to be finished so that should help with keeping things up to date. |
Minor changes to previous request #24, including