Skip to content

Conversation

@JiayiFeng
Copy link
Collaborator

@JiayiFeng JiayiFeng commented Aug 29, 2017

Related: #3684
Here may be easier to read.

Some part of batch_norm_op design is strongly related with Python API and needs future discussions.

use_global_est = False,
epsilon = 1e-6,
momentum = 0.99):
mean_cache = scope.new_var(name = 'estimated_mean', trainable = False)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There might be more than one batch_norm_op in the same topology, make sure variables like mean_cache and others have unique names?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make sure variables such as mean_cache invisible to other operators, or may be two operators write it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Yes. we need to make sure mean_cache has the unique name.
  2. mean_cache is defined inside def batch_norm_layer, could this make sure it's invisible for other operators?

# ...
```

`is_infer` is an attribute. Once an operator is created, its attributes can not be changed. It suggests us that we shall maintain two `batch_norm_op` in the model, one's `is_infer` is `True`(we call it `infer_batch_norm_op`) and the other one's is `False`(we call it `train_batch_norm_op`). They share all parameters and variables. How to organize them is related with Python API design, so I leave it here for further discussion.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, different Block shares the same operator objects. Did this require that different blocks has their own operator objects?

Add some details here?

Copy link
Collaborator Author

@JiayiFeng JiayiFeng Aug 30, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do need two distinct batch_norm_op. And it seems really require blocks hold their own operator objects... We shall have more discussion on it.

@JiayiFeng
Copy link
Collaborator Author

This design is temporarily placed on hold for it's strongly related to Python API design.

Copy link
Member

@jacquesqiao jacquesqiao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@jacquesqiao jacquesqiao merged commit af215a1 into PaddlePaddle:develop Oct 18, 2017
@JiayiFeng JiayiFeng deleted the dev_batch_norm branch October 18, 2017 21:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants