Skip to content

Provide more appropriate naming for the fourth and fifth arguments of BN #184

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Jun 20, 2019

Conversation

ir5
Copy link
Contributor

@ir5 ir5 commented Jun 19, 2019

Ad-hoc approach

@codecov-io
Copy link

codecov-io commented Jun 19, 2019

Codecov Report

Merging #184 into master will decrease coverage by 0.3%.
The diff coverage is 94.28%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #184      +/-   ##
==========================================
- Coverage   89.49%   89.18%   -0.31%     
==========================================
  Files          24       24              
  Lines        1361     1350      -11     
==========================================
- Hits         1218     1204      -14     
- Misses        143      146       +3
Impacted Files Coverage Δ
onnx_chainer/graph.py 95.55% <ø> (-0.05%) ⬇️
onnx_chainer/export.py 89.32% <100%> (-1.74%) ⬇️
onnx_chainer/context.py 97.36% <100%> (+1.07%) ⬆️
onnx_chainer/functions/connection.py 89.13% <100%> (-0.46%) ⬇️
onnx_chainer/functions/math.py 85.5% <100%> (-0.98%) ⬇️
onnx_chainer/functions/pooling.py 83.13% <100%> (-0.59%) ⬇️
onnx_chainer/functions/array.py 90.33% <86.66%> (+0.29%) ⬆️
onnx_chainer/functions/normalization.py 88.46% <91.66%> (-2.45%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7a54e08...878e763. Read the comment docs.

Copy link
Member

@shinh shinh left a comment

Choose a reason for hiding this comment

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

Defer to kusumoto-san and tanaka-san


To be converted as ONNX tensor.

Return:
Copy link
Member

Choose a reason for hiding this comment

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

Returns

Copy link
Contributor Author

Choose a reason for hiding this comment

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

modified.

@@ -10,34 +10,50 @@
@support((1, 6, 7))
def convert_BatchNormalization(func, opset_version, input_names,
output_names, context, parameters):
names = [context.get_name(v.get_variable().array) for v in func.inputs]
if names[1].startswith('param_'):
prefix = names[1][:-6] # remove "_gamma"
Copy link
Member

Choose a reason for hiding this comment

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

[:-1-len('_gamma')]?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems that this part will be entirely changed in #185. I will keep this part as is in this PR.

if names[1].startswith('param_'):
prefix = names[1][:-6] # remove "_gamma"
elif names[2].startswith('param_'):
prefix = names[2][:-5] # remove "_beta"
Copy link
Member

Choose a reason for hiding this comment

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

ditto?

@disktnk disktnk merged commit 878e763 into chainer:master Jun 20, 2019
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.

4 participants