tensorflow / tensorflow/probability
What is the rationale behind the current implementation of default_multivariate_normal_fn?
Nobody has claimed this yet.
- Dominant language
- Jupyter Notebook
- Stars
- 4.4k
- Forks
- 1.1k
- PR merge metrics
- No merged PRs in 30d
Description
The current implementation of default_multivariate_normal_fn looks like (excluding doc-strings)
def default_multivariate_normal_fn(dtype, shape, name, trainable, add_variable_fn):
del name, trainable, add_variable_fn # unused
dist = normal_lib.Normal(loc=tf.zeros(shape, dtype), scale=dtype.as_numpy_dtype(1))
batch_ndims = tf.size(dist.batch_shape_tensor())
return independent_lib.Independent(dist, reinterpreted_batch_ndims=batch_ndims)
If name, trainable and add_variable_fn are unused, why do you even require them to be passed as arguments? Furthermore, you initialize the scale parameter of the Normal distribution with dtype.as_numpy_dtype(1), which is as weird as it can be.
Why not simply have a method that returns a Normal initialized with 0s as means and 1s as scales, and require only the shape of that distribution?
Also, why does this function even exist, if you already have default_mean_field_normal_fn, which returns a closure def _fn(dtype, shape, name, trainable, add_variable_fn), which thus has the same parameters as default_multivariate_normal_fn and does more or less the same thing (apart from the fact that _fn uses its parameters to define the loc and scale, rather than deleting more than half of them). For consistency, wouldn't it be better to just use default_mean_field_normal_fn to also initialize the prior (by providing default parameters when calling it, or maybe have an alias, but not another function that does the same thing), or am I missing something?
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with tensorflow_probability/python/layers/util.py at default_multivariate_normal_fn and compare it with default_mean_field_normal_fn. Trace the callers and API expectations for these callbacks, then document whether their separate signatures and behavior are intentional or whether the proposed consolidation is appropriate.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python, tensorflow
- Domain
- machine-learning
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 18/100