tensorflow / tensorflow/probability

What is the rationale behind the current implementation of default_multivariate_normal_fn?

Open
#775 1 comment 0 reactions 0 assignees View on GitHub

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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.