Skip to content
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

Change the inference example to an unittest #7874

Merged
merged 10 commits into from
Jan 31, 2018

Conversation

Xreki
Copy link
Contributor

@Xreki Xreki commented Jan 25, 2018

fix #7876

@Xreki Xreki added the 预测 原名Inference,包含Capi预测问题等 label Jan 25, 2018
@Xreki Xreki force-pushed the core_add_inference_unittest branch from 460854e to 20d3af6 Compare January 25, 2018 11:14
@Xreki Xreki changed the title [WIP] Change the inference example to an unittest Change the inference example to an unittest Jan 26, 2018
@Xreki Xreki force-pushed the core_add_inference_unittest branch from d021c86 to eca58a6 Compare January 26, 2018 09:07
if args.nn_type == 'mlp':
tensor_img = numpy.random.rand(1, 28, 28).astype("float32")
else:
tensor_img = numpy.random.rand(1, 1, 28, 28).astype("float32")
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are the numpy array dimensions different for 'mlp' and 'conv_net' ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because the input's dimension of conv_op should be 4-D or 5-D, and the input's dimension of mul_op will be flattened into 2-D. So, we can use line 169 to simplify the code.

kexinzhao
kexinzhao previously approved these changes Jan 30, 2018
Copy link
Contributor

@kexinzhao kexinzhao left a comment

Choose a reason for hiding this comment

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

LGTM!

Maybe wait for @luotao1 and @sidgoyal78 to also have a look?

A separate question would be "are we going to merge this before #7690"?

@Xreki Xreki force-pushed the core_add_inference_unittest branch from 3c56151 to f5d9336 Compare January 30, 2018 11:07
Copy link
Contributor

@sidgoyal78 sidgoyal78 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. I have one suggestion: for adding the inference example in the python side, can we create another folder?
I remember the test_recognize_digits.py was modified significantly by someone few days back (and in that process the inference parts were simply deleted). So maybe just to avoid general conflict, we can do some code duplication in order to keep things independent?

@Xreki Xreki force-pushed the core_add_inference_unittest branch from fefe426 to f5990b4 Compare January 31, 2018 02:25
Copy link
Contributor

@luotao1 luotao1 left a comment

Choose a reason for hiding this comment

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

LGTM

@luotao1 luotao1 merged commit 455639b into PaddlePaddle:develop Jan 31, 2018
@Xreki
Copy link
Contributor Author

Xreki commented Jan 31, 2018

for adding the inference example in the python side, can we create another folder?

If doing this, there will be so many redundant codes. All the network will be write twice and train twice, because we need to use the training results.

I remember the test_recognize_digits.py was modified significantly by someone few days back (and in that process the inference parts were simply deleted).

It won't happen again, because I change the inference example to an unittest. If someone deletes the python codes again, the CI will fail.

@Xreki Xreki deleted the core_add_inference_unittest branch November 14, 2018 02:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
预测 原名Inference,包含Capi预测问题等
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Change the inference example to an unitest test_inference_recognize_digits
4 participants