Skip to content

fix assertion failure issue when test_analyzer_bert uses ngraph#17148

Merged
baojun-nervana merged 1 commit intoPaddlePaddle:developfrom
LeoZhao-Intel:ngraph_bert
Apr 29, 2019
Merged

fix assertion failure issue when test_analyzer_bert uses ngraph#17148
baojun-nervana merged 1 commit intoPaddlePaddle:developfrom
LeoZhao-Intel:ngraph_bert

Conversation

@LeoZhao-Intel
Copy link
Contributor

resolve #17147
test=develop

@LeoZhao-Intel
Copy link
Contributor Author

@baojun-nervana
Hi baojun, please help to review the patch, I am just trying to run bert using ngraph and find this issue, I guess it should be a typo.

@luotao1
Copy link
Contributor

luotao1 commented Apr 29, 2019

How about add nGraph test in test_analyzer_bert to check this PR?

@LeoZhao-Intel
Copy link
Contributor Author

LeoZhao-Intel commented Apr 29, 2019

How about add nGraph test in test_analyzer_bert to check this PR?

I am ok. may need baojun's opinion as well. @baojun-nervana

Copy link
Contributor

@baojun-nervana baojun-nervana left a comment

Choose a reason for hiding this comment

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

LGTM. thanks.

@baojun-nervana baojun-nervana merged commit bf4b21f into PaddlePaddle:develop Apr 29, 2019
@luotao1
Copy link
Contributor

luotao1 commented Apr 30, 2019

@baojun-nervana Please do not merge it yourself.

How about add nGraph test in test_analyzer_bert to check this PR?

how do you think about: #17148 (comment)

@baojun-nervana
Copy link
Contributor

@luotao1 We are still validating bert model. We may not want to turn it on for bert model now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test_analyzer_bert runs into assertion failure when ngraph is used

3 participants