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

[Bug] PaddlePaddle integration tests busted at main #9231

Closed
areusch opened this issue Oct 8, 2021 · 13 comments
Closed

[Bug] PaddlePaddle integration tests busted at main #9231

areusch opened this issue Oct 8, 2021 · 13 comments

Comments

@areusch
Copy link
Contributor

areusch commented Oct 8, 2021

We temporarily had a problem with our post-merge CI builds, so apologies this wasn't caught quite as quickly as we normally would. Seems that 5 integration tests related to PaddlePaddle are busted at main likely due to a merge conflict.

https://ci.tlcpack.ai/blue/organizations/jenkins/tvm/detail/main/1883/pipeline

FAILED tests/python/driver/tvmc/test_compiler.py::test_compile_paddle_module
FAILED tests/python/driver/tvmc/test_compiler.py::test_cross_compile_aarch64_paddle_module
FAILED tests/python/driver/tvmc/test_compiler.py::test_cross_compile_options_aarch64_paddle_module
FAILED tests/python/driver/tvmc/test_frontends.py::test_load_model__paddle - ...
FAILED tests/python/driver/tvmc/test_tvmc_common.py::test_compile_paddle_module_nchw_to_nhwc

I suspect #9083 might be related. Is anyone more familiar with it able to look? cc @jiangjiajun @masahi @leandron

@masahi
Copy link
Member

masahi commented Oct 8, 2021

I think this paddle PR merged yesterday is related #9126

Since #9083 was merged three days ago and we had no problem until yesterday. And there was about three days between the last CI job for 9126 ran before it was merged yesterday.

@masahi
Copy link
Member

masahi commented Oct 8, 2021

This is also blocking PR jobs, https://ci.tlcpack.ai/blue/organizations/jenkins/tvm/detail/PR-9230/1/pipeline, so I'm going to send a revert. The question is, which of #9083 or #9126 we should revert?

cc @jiangjiajun @junrushao1994

@junrushao
Copy link
Member

I have no idea why it happens, and no objection for a revert

@junrushao
Copy link
Member

What about we try to revert #9083 first? Looks like the unittests came from that PR

@masahi
Copy link
Member

masahi commented Oct 8, 2021

Yeah, #9083 was merged three days ago and as far as I know, there have been no problem with that tests until yesterday. My hunch is that #9126, which apparently didn't run CI job after #9083 was merged, broke it.

@masahi
Copy link
Member

masahi commented Oct 8, 2021

Sent a revert for #9126 for now. We can decide later if we want to merge that or not.

@junrushao
Copy link
Member

Sounds good to me!

@masahi
Copy link
Member

masahi commented Oct 8, 2021

#9232 is looking good

@jiangjiajun
Copy link
Contributor

I just saw this issue, I'll try to reproduce this problem now

@jiangjiajun
Copy link
Contributor

jiangjiajun commented Oct 9, 2021

I have reproduced the problem, and send a pull request #9236 to fix the bug, let's see if this could solve the ci problem @masahi @junrushao1994 @areusch

@jiangjiajun
Copy link
Contributor

I have reproduced the problem, and send a pull request #9236 to fix the bug, let's see if this could solve the ci problem @masahi @junrushao1994 @areusch

Now all the ci test passed, sorry for all the blocking PRs, we can merge #9236 and retrigger ci for the blocked pull request.

The reason this bug not detected by ci is,

  1. Add TVMC Frontend for PaddlePaddle #9083 was merged at 10/05
  2. [Frontend][PaddlePaddle] Add 10+ operators for PaddlePaddle #9126 passed all the ci test at 10/04, merged at 10/08

this caused #9126 didn't test with #9083

@junrushao
Copy link
Member

Thanks @jiangjiajun for the quick response! I merged the hotfix PR just now. Let's see what will happen on mainline

@masahi
Copy link
Member

masahi commented Oct 10, 2021

We can close this now, thanks @jiangjiajun

@masahi masahi closed this as completed Oct 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants