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

fix segmentation testing and support CPU #888

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

wkcn
Copy link
Member

@wkcn wkcn commented Jul 29, 2019

Hi, there.

In the testing script of segmentation, the shape of predict[0] is (num_cls, height, weight).

In the line 71 of scripts/segmentation/test.py, the axis of mx.nd.argmax should be 0.

Besides, I modify the code to support the inference of segmentation model on CPU.

Thank you!

@mli
Copy link
Member

mli commented Jul 29, 2019

Job PR-888-1 is done.
Docs are uploaded to http://gluon-vision-staging.s3-website-us-west-2.amazonaws.com/PR-888/1/index.html
Code coverage of this PR: pr.svg vs. Master: master.svg

@mli
Copy link
Member

mli commented Jul 29, 2019

Job PR-888-2 is done.
Docs are uploaded to http://gluon-vision-staging.s3-website-us-west-2.amazonaws.com/PR-888/2/index.html
Code coverage of this PR: pr.svg vs. Master: master.svg

@hetong007 hetong007 requested a review from zhanghang1989 July 29, 2019 21:40
@@ -68,14 +68,14 @@ def test(args):
im_paths = dsts
predicts = evaluator.parallel_forward(data)
for predict, impath in zip(predicts, im_paths):
predict = mx.nd.squeeze(mx.nd.argmax(predict[0], 1)).asnumpy() + \
predict = mx.nd.squeeze(mx.nd.argmax(predict[0], 0)).asnumpy() + \
Copy link
Contributor

Choose a reason for hiding this comment

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

Following the standard practice, we use the first dimension as Batch dimension by default. Could you please still use dim=1.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed it. : )

Copy link
Contributor

@zhanghang1989 zhanghang1989 left a comment

Choose a reason for hiding this comment

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

Could you please address my comment

@mli
Copy link
Member

mli commented Jul 30, 2019

Job PR-888-3 is done.
Docs are uploaded to http://gluon-vision-staging.s3-website-us-west-2.amazonaws.com/PR-888/3/index.html
Code coverage of this PR: pr.svg vs. Master: master.svg

@@ -68,14 +68,14 @@ def test(args):
im_paths = dsts
predicts = evaluator.parallel_forward(data)
for predict, impath in zip(predicts, im_paths):
predict = mx.nd.squeeze(mx.nd.argmax(predict[0], 1)).asnumpy() + \
testset.pred_offset
predict = mx.nd.squeeze(mx.nd.argmax(predict, 1), axis=0).\
Copy link
Contributor

Choose a reason for hiding this comment

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

This will break existing segmentation model, which outputs a list by default.

Copy link
Member Author

@wkcn wkcn Aug 2, 2019

Choose a reason for hiding this comment

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

What are the elements in the list predict?
I think predicts is a list of NDArray, and predict is a NDArray whose shape is (1, num_cls, height, width)

Copy link
Contributor

Choose a reason for hiding this comment

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

predicts is a list of list of NDArray

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you! I will check it.

Copy link
Member Author

Choose a reason for hiding this comment

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

@zhanghang1989
Hi, I found the type of predicts is tuple, and the type of predict is NDArray.

Test script: python test.py --dataset pascal_voc --model-zoo psp_resnet101_voc

@wuxun-zhang
Copy link
Collaborator

Please rebase the code to resolve conflicts since this PR has been merged into master.

@mli
Copy link
Member

mli commented Aug 20, 2019

Job PR-888-4 is done.
Docs are uploaded to http://gluon-vision-staging.s3-website-us-west-2.amazonaws.com/PR-888/4/index.html
Code coverage of this PR: pr.svg vs. Master: master.svg

@@ -194,6 +194,7 @@ def benchmarking(model, args):

if __name__ == "__main__":
args = parse_args()
args.test_batch_size = max(1, args.ngpus)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just want to know why args.test_batch_size here is dependent on args.ngpus. Also since args.batch_size is already defined, is it possible to use this one directly instead of adding a new one like test_batch_size?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you for the review! I have removed this line, and I will update the code later.
The test_batch_size is defined in train.py, and it is the batch size of testing dataset. For testing script, we should keep the consistency with training procedure.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you for the review! I have removed this line, and I will update the code later.
The test_batch_size is defined in train.py, and it is the batch size of testing dataset. For testing script, we should keep the consistency with training procedure.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't know why the program gets stuck. I will check it later.

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