blueoil icon indicating copy to clipboard operation
blueoil copied to clipboard

tf_upgrade_v2 for network/classification/vgg16.py

Open ytfksw opened this issue 4 years ago • 9 comments

What this patch does to fix the issue.

I converted the blueoil/network/classification/vgg16.py with the command line tool tf_upgrade_v2 to run on tensorflow2.

Here's what I did:

  1. install newest tenosrflow (tensorflow-gpu==1.5.2)
  2. Run the upgrade script tf_upgrade_v2
  3. Check the upgrade report for warnings and errors
  4. Run test (make test)

Link to any relevant issues or pull requests.

  • #479
  • https://www.tensorflow.org/guide/upgrade

ytfksw avatar Jun 04 '20 09:06 ytfksw

This PR needs Approvals as follows.

  • Ownership Approval for / from iizukak, tkng, ruimashita
  • Readability Approval for Python from tkng, tsawada, tfujiwar

Please choose reviewers and requet reviews!

Click to see how to approve each reviews

You can approve this PR by triggered comments as follows.

  • Approve all reviews requested to you (readability and ownership) and LGTM review Approval, LGTM

  • Approve all ownership reviews Ownership Approval or OA

  • Approve all readability reviews Readability Approval or RA

  • Approve specified review targets

    • Example of Ownership Reviewer of /: Ownership Approval for / or OA for /
    • Example of Readability Reviewer of Python: Readability Approval for Python or RA for Python
  • Approve LGTM review LGTM

See all trigger comments

Please replace [Target] to review target

  • Ownership Approval
    • Ownership Approval for [Target]
    • OA for [Target]
    • Ownership Approval
    • OA
    • Approval
  • Readability Approval
    • Readability Approval for [Target]
    • RA for [Target]
    • [Target] Readability Approval
    • [Target] RA
    • Readability Approval
    • RA
    • Approval
  • LGTM
    • LGTM
    • lgtm

bo-code-review-bot[bot] avatar Jun 04 '20 09:06 bo-code-review-bot[bot]

This is the whole convert report:

TensorFlow 2.0 Upgrade Script
-----------------------------
Converted 1 files
Detected 0 issues that require attention
--------------------------------------------------------------------------------
================================================================================
Detailed log follows:

================================================================================
--------------------------------------------------------------------------------
Processing file 'vgg16.py'
 outputting to 'vgg16.py'
--------------------------------------------------------------------------------

30:20: INFO: Added keywords to args of function 'tf.cond'
63:20: INFO: Changing keep_prob arg of tf.nn.dropout to rate, and recomputing value.

66:20: INFO: Changing keep_prob arg of tf.nn.dropout to rate, and recomputing value.

84:29: INFO: Changing tf.contrib.layers xavier initializer to a tf.compat.v1.keras.initializers.VarianceScaling and converting arguments.

85:29: INFO: tf.zeros_initializer requires manual check. Initializers no longer have the dtype argument in the constructor or partition_info argument in the __call__ method.
The calls have been converted to compat.v1 for safety (even though they may already have been correct).
85:29: INFO: Renamed 'tf.zeros_initializer' to 'tf.compat.v1.zeros_initializer'
110:29: INFO: Changing tf.contrib.layers xavier initializer to a tf.compat.v1.keras.initializers.VarianceScaling and converting arguments.

111:29: INFO: tf.zeros_initializer requires manual check. Initializers no longer have the dtype argument in the constructor or partition_info argument in the __call__ method.
The calls have been converted to compat.v1 for safety (even though they may already have been correct).
111:29: INFO: Renamed 'tf.zeros_initializer' to 'tf.compat.v1.zeros_initializer'
--------------------------------------------------------------------------------

ytfksw avatar Jun 04 '20 09:06 ytfksw

tf_upgrade_v2 sometimes changed argument to keyword argument. I adjusted the keyword argument (39cb96b), because some arguments become meaninglessly long.

Because API has changed by changing from tf1 to tf2. It seems to have been converted to a keyword argument to avoid misunderstanding.

tf.cond tf1: https://www.tensorflow.org/versions/r1.15/api_docs/python/tf/cond

tf.cond(
    pred,
    true_fn=None,
    false_fn=None,
    strict=False,
    name=None,
    fn1=None,
    fn2=None
)

tf2: https://www.tensorflow.org/api_docs/python/tf/cond

tf.cond(
    pred, true_fn=None, false_fn=None, name=None
)

ytfksw avatar Jun 04 '20 09:06 ytfksw

I did manual adjustments because of the tf_upgrage_v2 bug.

Conversion of tf_upgrage_v2

https://github.com/blue-oil/blueoil/pull/1081/commits/abc88c9e61ca9cd91778cf6ad5bccbabc949001e#diff-8ba3a27cc98c470fa86a7d5bac9faaaeL30-R30

tf1:https://www.tensorflow.org/versions/r1.15/api_docs/python/tf/nn/dropout

tf.nn.dropout(
    x,
    keep_prob=None,
    noise_shape=None,
    seed=None,
    name=None,
    rate=None
)

tf2:https://www.tensorflow.org/api_docs/python/tf/nn/dropout

tf.nn.dropout(
    x, rate, noise_shape=None, seed=None, name=None
)

In the change from tf1 to tf2, keep_prob is now deprecated and rate is now used. Teh relationship is rate = 1 - keep_prob.

When we run tf_upgrade_v2, the tool should keep tf1 and tf2 compatible, but it doesn't work well...

In this case, by giving the rate keyword as the second argument, we can convert correctly.

Manual adjustment https://github.com/blue-oil/blueoil/pull/1081/commits/cadcd45f4cff302b30d6ab77b3435ab474009273

I am considering reporting to the tensorflow official.

ytfksw avatar Jun 05 '20 02:06 ytfksw

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Jun 12 '20 06:06 CLAassistant

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

CLAassistant avatar Jun 12 '20 06:06 CLAassistant

This is an automatic conversion with tf_upgrade_v2. I also checked directly and confirmed that they are identical.

The following changes have been made https://github.com/blue-oil/blueoil/pull/1081/files/cadcd45f4cff302b30d6ab77b3435ab474009273#diff-8ba3a27cc98c470fa86a7d5bac9faaaeL84-R88

tf.contrib.layers.xavier_initializer

The implementation of tf.contrib.layers.xavier_initializer is this https://github.com/tensorflow/tensorflow/blob/r1.8/tensorflow/contrib/layers/python/layers/initializers.py#L31-L57

Docstring explaines as follows.

This initializer is designed to keep the scale of The gradients roughly the
In uniform distribution this It ends up being the range:
`x = sqrt(6. / (in + out)); [-x, x]` and for normal distribution a standard
deviation of `sqrt(2. / (in + out))` is used.

Namely, the distribution is as follows:

  • uniform distribution
  • range: [-sqrt(6. / (in + out)), sqrt(6. / (in + out)) ))]]
    • In this case, the standard deviation is sqrt(2. / ( in + out))

In this function, tf.contrib.layers.variance_scaling with option factor=1.0, mode='FAN_AVG', uniform =uniform is called.

This is a xavier_initializer. See also.

tf.compat.v1.keras.initializers. VarianceScaling

tf.compat.v1.keras.initializers.VarianceScaling is not an alias of tf.contrib.layers.variance_scaling_initializer, but a compatible function.

So tf.compat.v1.keras.initializers.VarianceScaling with same option scale=1.0, mode="fan_avg", distribution="uniform" is a xavier_initializer.

ytfksw avatar Jun 17 '20 05:06 ytfksw

what's the status of this PR? @iizukak Can somebody else review if you're busy?

tsawada avatar Jul 22 '20 09:07 tsawada

@iizukak Could you review it?

ytfksw avatar Sep 02 '20 08:09 ytfksw