Skip to content

Disable one more test when contrib ops are disabled. #789

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 1 commit into from
Apr 8, 2019

Conversation

pranavsharma
Copy link
Contributor

One more test failed as per https://aiinfra.visualstudio.com/Lotus/_build/results?buildId=79109. Not sure why this didn't fail earlier.

@pranavsharma pranavsharma requested a review from a team as a code owner April 7, 2019 05:01
@snnn
Copy link
Member

snnn commented Apr 7, 2019

Maybe I know. There was a known issue in onnx_test_runner. It was introduced in Pranav's PR #639, that it may access uninitialized memory when retrieving test config. It because OnnxTestCase::ParseConfig() may forget to initialize some config variable, especially post_processing_. If it is true, value will be cut into [0-255].
The bug should already be fixed in #751

@jignparm jignparm merged commit 18ca4df into rel-0.3.1 Apr 8, 2019
@jignparm jignparm deleted the disable_test_contrib_ops branch April 8, 2019 00:36
@jignparm
Copy link
Contributor

jignparm commented Apr 8, 2019

@snnn thanks for the the links above. The changes from #639 should be available in rel-0.3.1 branch. The failing tests seem to be transient. Merging this PR into rel-0.3.1 is building successfully.

@snnn
Copy link
Member

snnn commented Apr 8, 2019

I don't think this PR is good. Because, you just exclude one test failure you have seen, but you may get more random test failures in future runs.

@pranavsharma
Copy link
Contributor Author

pranavsharma commented Apr 8, 2019

As you said it'll get fixed in the master branch due to PR 751.

@snnn
Copy link
Member

snnn commented Apr 8, 2019

If you have more changes in rel-0.3.1, you may hit this issue again.

@pranavsharma
Copy link
Contributor Author

rel-0.3.1 is sealed now.

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