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

Eltwise shared test refactor #645

Closed

Conversation

kmagiers
Copy link
Contributor

@kmagiers kmagiers commented May 27, 2020

Removal of Eltwise test
Extension of Add test
Extension of Multiply test
Introduction of Subtract test

@kmagiers kmagiers requested review from iefode, esmirno, dorloff, pavel-rodionov and a team May 27, 2020 15:17
@pavel-rodionov pavel-rodionov added the category: IE Tests OpenVINO Test: plugins and common label May 29, 2020
const std::vector<InferenceEngine::Precision> netPrecisions = {
InferenceEngine::Precision::FP32,
InferenceEngine::Precision::FP16
std::vector<std::vector<std::vector<size_t>>> inputShapes = { {std::vector<std::size_t>({1, 30})} };
Copy link
Contributor

Choose a reason for hiding this comment

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

As in test for Multiply layer we can also add more input shapes here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I'll add additional shapes, I simply left the "original" shapes of the test

Copy link
Contributor

@pavel-rodionov pavel-rodionov left a comment

Choose a reason for hiding this comment

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

Looks good to me

@pavel-rodionov
Copy link
Contributor

Also could you please re-name your PR so it won't be confusing. Because you are not just removing test for Eltwise layer - your are replacing it for another tests that are actually better.

@kmagiers kmagiers changed the title Eltwise shared test removal Eltwise shared test refactor Jun 1, 2020
@@ -0,0 +1,71 @@
// Copyright (C) 2020 Intel Corporation
Copy link
Contributor

Choose a reason for hiding this comment

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

@dmitry-gorokhov Please review updated/added CPU plugin instances

Copy link
Contributor

@iefode iefode Jun 1, 2020

Choose a reason for hiding this comment

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

After discussion with @dmitry-gorokhov, we have take a decision to make single eltwise tests file instead of existing. Dmitry had more powerful argument in our discussion.

I will use this PR in merge of these tests. Sorry for inconvenience.

So, This PR is not to be merged.

Copy link
Contributor

Choose a reason for hiding this comment

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

@iefode, can you please elaborate on "I will use this PR in merge of these tests" - what do you mean?

Copy link
Contributor

Choose a reason for hiding this comment

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

I will refactor these chanes in the refactoring

Copy link
Contributor

Choose a reason for hiding this comment

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

Refactor in the refactoring? It's still unclear. Is anything expected from our side?

Copy link
Contributor

Choose a reason for hiding this comment

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

I will do it myself. I don't expected some actions from your side

@kmagiers kmagiers force-pushed the Eltwise_shared_test_removal branch from 25f636c to 421f83d Compare June 1, 2020 14:17
@iefode
Copy link
Contributor

iefode commented Jun 2, 2020

@kmagiers @dorloff Sorry for inconvenience again.
PR: #726

@iefode
Copy link
Contributor

iefode commented Jun 10, 2020

PR: #726 merged to the master. This PR should be closed

@iefode iefode closed this Jun 10, 2020
redradist pushed a commit to redradist/openvino that referenced this pull request Oct 6, 2023
* [NVIDIA] Fix CudaUnitTests target for Windows

* [NVIDIA] Fix CUDA::math::pow function

* [NVIDIA] Fix Windows build error in convert_benchmark.cpp

---------

Co-authored-by: Nadezhda Ageeva <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: IE Tests OpenVINO Test: plugins and common
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants