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

[VPU][TESTS]Fix myriad tests on MacOS #3681

Merged
merged 2 commits into from
Dec 30, 2020

Conversation

PoliOwl
Copy link
Contributor

@PoliOwl PoliOwl commented Dec 18, 2020

fix for myriadTestsTopK_smoke and smoke_DynamicSqueeze/DSR_Squeeze for MacOS
disable deprecated myriadLayersTestsExpGenerateProposals_smoke for macOS

@PoliOwl PoliOwl requested a review from a team December 18, 2020 15:12
@PoliOwl PoliOwl requested a review from a team as a code owner December 18, 2020 15:12
@PoliOwl PoliOwl requested a review from a user December 21, 2020 07:57
Copy link
Contributor

@Maxim-Doronin Maxim-Doronin left a comment

Choose a reason for hiding this comment

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

Looks ok
I think the behavior was not changed, but it is still unclear to me why the previous version had not worked on macOS

@PoliOwl PoliOwl force-pushed the fixMyriadTestsOnMacOS branch from fc2de7c to f8cae70 Compare December 21, 2020 09:42
@Maxim-Doronin Maxim-Doronin self-assigned this Dec 21, 2020
@PoliOwl PoliOwl force-pushed the fixMyriadTestsOnMacOS branch 2 times, most recently from 45f6bd6 to 90b4f98 Compare December 22, 2020 09:32
@@ -24,7 +24,7 @@ void dynamicToStaticShapeSqueeze(std::shared_ptr<ngraph::Node> target) {
target, ngraph::opset3::Squeeze::type_info);

const auto dsr = target->input_value(0).get_node_shared_ptr();
VPU_THROW_UNLESS(std::dynamic_pointer_cast<ngraph::vpu::op::DynamicShapeResolver>(dsr),
VPU_THROW_UNLESS(ngraph::is_type<ngraph::vpu::op::DynamicShapeResolver>(dsr),
Copy link
Contributor

Choose a reason for hiding this comment

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

Why dynamic_pointer_cast does not work?

CC @ilyachur

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like it is an internal VPU node has some issues with RTTI?
Am I right? Or in other case should dynamic_pointer_cast and is_type should produce the same result.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It looks like this problem only occurs on MacOS, and cases segmentation fault, but is_type works fine
I've tried to directly turn on RTTI, but it didn't help

Copy link
Contributor

Choose a reason for hiding this comment

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

RTTI is enabled by default. We had the same problems when symbols aren't exported from the library. I am not sure that it is your case, need to check.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How did you discovered your problem?

Copy link
Contributor

Choose a reason for hiding this comment

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

@PoliOwl the issue usually happens when you use dynamic cast to a type which is located in a different shared library (in this case class must be export from that library). E.g. I believe if the issue comes when you cast to ngraph type which is not exported, but these types are located inside your library and the issue should not happen...

Try to build with clang and -fsanitize=vptr (see https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html) and run this test.

@@ -140,7 +140,11 @@ static void genInputs(InferenceEngine::BlobMap inputMap,
inputIMinfo[1] = PrecisionUtils::f32tof16( (float) imgW );
}

#ifdef __APPLE__
TEST_P(myriadLayersTestsExpGenerateProposals_smoke, DISABLED_ExpGenerateProposals) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did you disable some tests for macOS? Is it related with RTTI issues?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This tests fails on MacOS, but this problem is depended on input data, which generates randomly, so it very hard to reproduce, and since those are deprecated tests, they were disabled. As far as I can tell they don't relate to the RTTI issues

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe some variable or class member isn't initialised and it causes the issue... Do you have a plan to move this test on new infrastructure and enable it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

there is plans for moving tests, but with a very low priority

@PoliOwl PoliOwl force-pushed the fixMyriadTestsOnMacOS branch from 90b4f98 to 715ad4a Compare December 24, 2020 09:57
Polina added 2 commits December 28, 2020 14:01
…henged dynamic_pointer_cast to ngraph::is_type at dynamic_to_static_shape_squeeze.cpp
@PoliOwl PoliOwl force-pushed the fixMyriadTestsOnMacOS branch from 715ad4a to ccab9d5 Compare December 28, 2020 11:03
@Maxim-Doronin
Copy link
Contributor

@PoliOwl, I'm merging the PR to fix functional tests on MacOS on VPU CI.
Please, continue investigating the topics from previous comments in this PR

@Maxim-Doronin Maxim-Doronin merged commit 0850479 into openvinotoolkit:master Dec 30, 2020
mryzhov pushed a commit to mryzhov/openvino that referenced this pull request Jan 14, 2021
* fix sorting ref for myriadTestsTopK_smoke on macOS and chenged dynamic_pointer_cast to ngraph::is_type at dynamic_to_static_shape_squeeze.cpp
* disable accuracy/myriadLayersTestsExpGenerateProposals_smoke.ExpGenerateProposals tests for macOS
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