dnn: fix HAVE_TIMVX macro definition in dnn test by fengyuentau · Pull Request #24425 · opencv/opencv

fengyuentau · 2023-10-18T08:14:29Z

Pull Request Readiness Checklist

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

I agree to contribute to the project under Apache 2 License.
To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV
The PR is proposed to the proper branch
There is a reference to the original bug report and related work
There is accuracy test, performance test and test data in opencv_extra repository, if applicable
Patch to opencv_extra has the same branch name.
The feature is well documented and sample code can be built with the project CMake

asmorkalov · 2023-10-18T08:36:07Z

modules/dnn/CMakeLists.txt

+ocv_option(OPENCV_TEST_DNN_TIMVX "Build test with TIM-VX" (TARGET ocv.3rdparty.timvx))
+if(TARGET ocv.3rdparty.timvx AND OPENCV_TEST_DNN_TIMVX)
+  if(TARGET opencv_test_dnn)
+    ocv_target_link_libraries(opencv_test_dnn ocv.3rdparty.timvx)


Why do you need it? It's internal detail of implementation and user code should not depend on it, including tests. libopencv_dnn should have dependency from TIMVX.

Test does not work with missing HAVE_TIMVX macro:

opencv/modules/dnn/test/test_int8_layers.cpp

Lines 11 to 22 in 240b245

testing::internal::ParamGenerator< tuple<Backend, Target> > dnnBackendsAndTargetsInt8()

{

std::vector< tuple<Backend, Target> > targets;

targets.push_back(make_tuple(DNN_BACKEND_OPENCV, DNN_TARGET_CPU));

#ifdef HAVE_TIMVX

targets.push_back(make_tuple(DNN_BACKEND_TIMVX, DNN_TARGET_NPU));

#endif

#ifdef HAVE_INF_ENGINE

targets.push_back(make_tuple(DNN_BACKEND_INFERENCE_ENGINE_NGRAPH, DNN_TARGET_CPU));

#endif

return testing::ValuesIn(targets);

}

Basically I need HAVE_TIMVX=1.

We don't need that.

There is registry API:

opencv/modules/dnn/include/opencv2/dnn/dnn.hpp

Lines 125 to 126 in 240b245

CV_EXPORTS std::vector< std::pair<Backend, Target> > getAvailableBackends();

CV_EXPORTS_W std::vector<Target> getAvailableTargets(dnn::Backend be);

And usage in tests:

opencv/modules/dnn/test/test_common.impl.hpp

Lines 291 to 296 in 240b245

if (withVkCom)

{

available = getAvailableTargets(DNN_BACKEND_VKCOM);

for (std::vector< Target >::const_iterator i = available.begin(); i != available.end(); ++i)

targets.push_back(make_tuple(DNN_BACKEND_VKCOM, *i));

}

DNN_BACKEND_TIMVX is not in opencv/modules/dnn/test/test_common.impl.hpp.

asmorkalov · 2023-10-19T05:57:33Z

The mentioned macro is used in tests in 2 places and both of them may be replaced with runtime check like Alexander suggested.

fengyuentau · 2023-10-19T06:59:19Z

So why these happen? See

opencv/modules/dnn/CMakeLists.txt

Lines 290 to 295 in 6e4280e

    
           ocv_option(OPENCV_TEST_DNN_OPENVINO "Build test with OpenVINO code" (TARGET ocv.3rdparty.openvino)) 
        
           if(TARGET ocv.3rdparty.openvino AND OPENCV_TEST_DNN_OPENVINO) 
        
             if(TARGET opencv_test_dnn) 
        
               ocv_target_link_libraries(opencv_test_dnn ocv.3rdparty.openvino) 
        
             endif() 
        
           endif()

and

opencv/modules/dnn/test/test_int8_layers.cpp

Lines 11 to 22 in 6e4280e

    
           testing::internal::ParamGenerator< tuple<Backend, Target> > dnnBackendsAndTargetsInt8() 
        
           { 
        
               std::vector< tuple<Backend, Target> > targets; 
        
               targets.push_back(make_tuple(DNN_BACKEND_OPENCV, DNN_TARGET_CPU)); 
        
           #ifdef HAVE_TIMVX 
        
               targets.push_back(make_tuple(DNN_BACKEND_TIMVX, DNN_TARGET_NPU)); 
        
           #endif 
        
           #ifdef HAVE_INF_ENGINE 
        
               targets.push_back(make_tuple(DNN_BACKEND_INFERENCE_ENGINE_NGRAPH, DNN_TARGET_CPU)); 
        
           #endif 
        
               return testing::ValuesIn(targets); 
        
           }

(see line 18).

The TIM-VX backend is only for int8-quantized models. dnnBackendsAndTargetsInt8 is only defined and used in modules/dnn/test/test_int8_layers.cpp.

fengyuentau · 2023-10-19T07:01:28Z

Also @asmorkalov could you provide a key via email for me to set up a GitHub Actions connection with my Khadas VIM3?

opencv-alalek · 2023-10-19T07:27:45Z

So why these happen? See

Because tests use OpenVINO API directly to test IR models (as a reference result).

opencv/modules/dnn/test/test_ie_models.cpp

Lines 185 to 210 in 6e4280e

    
           void runIE(Target target, const std::string& xmlPath, const std::string& binPath, 
        
                      std::map<std::string, cv::Mat>& inputsMap, std::map<std::string, cv::Mat>& outputsMap) 
        
           { 
        
               SCOPED_TRACE("runIE"); 
        
               std::string device_name; 
        
           #if defined(INF_ENGINE_RELEASE) && INF_ENGINE_VER_MAJOR_GT(2019010000) 
        
               Core ie; 
        
           #else 
        
               InferenceEnginePluginPtr enginePtr; 
        
               InferencePlugin plugin; 
        
           #endif 
        
           #if defined(INF_ENGINE_RELEASE) && INF_ENGINE_VER_MAJOR_GT(2019030000) 
        
               CNNNetwork net = ie.ReadNetwork(xmlPath, binPath); 
        
           #else 
        
               CNNNetReader reader; 
        
               reader.ReadNetwork(xmlPath); 
        
               reader.ReadWeights(binPath); 
        
               CNNNetwork net = reader.getNetwork(); 
        
           #endif 
        
               ExecutableNetwork netExec; 
        
               InferRequest infRequest;

Tests doesn't use special TIM-VX API

opencv-alalek · 2023-10-20T06:44:19Z

modules/dnn/CMakeLists.txt

+ocv_option(OPENCV_TEST_DNN_TIMVX "Build test with TIM-VX" (HAVE_TIMVX))
+if(OPENCV_TEST_DNN_TIMVX)
+  if(TARGET opencv_test_dnn)
+  ocv_target_compile_definitions(opencv_test_dnn PRIVATE "HAVE_TIMVX=1")


indentation

Thank you for finding this! Meanwhile please hold merging this PR until the hardware is in the CI running the tests.

fengyuentau · 2023-10-20T10:27:46Z

The log shows tim-vx backend works fine with this patch, https://github.com/opencv/opencv/actions/runs/6586141510/job/17893901244?pr=24425

Ready to be merged once all tests are done.

asmorkalov · 2023-10-20T10:43:08Z

@fengyuentau I see VIM3 build passed on CI. Is it ready? May I merge the pr?

asmorkalov

👍

fengyuentau · 2023-10-20T10:44:50Z

@fengyuentau I see VIM3 build passed on CI. Is it ready? May I merge the pr?

Yes, it is ready and you can merge it.

dnn: fix HAVE_TIMVX macro definition in dnn test opencv#24425 ### Pull Request Readiness Checklist See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request - [x] I agree to contribute to the project under Apache 2 License. - [x] To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV - [x] The PR is proposed to the proper branch - [x] There is a reference to the original bug report and related work - [x] There is accuracy test, performance test and test data in opencv_extra repository, if applicable Patch to opencv_extra has the same branch name. - [x] The feature is well documented and sample code can be built with the project CMake

fix HAVE_TIMVX macro in dnn test

41e08bd

fengyuentau added category: dnn category:dnn_timvx TIM-VX related issues in DNN module labels Oct 18, 2023

fengyuentau added this to the 4.9.0 milestone Oct 18, 2023

fengyuentau assigned asmorkalov Oct 18, 2023

asmorkalov reviewed Oct 18, 2023

View reviewed changes

another solution

269e59d

opencv-alalek approved these changes Oct 20, 2023

View reviewed changes

fix indentation

2bc9aa5

asmorkalov approved these changes Oct 20, 2023

View reviewed changes

asmorkalov merged commit 996b6c3 into opencv:4.x Oct 20, 2023

fengyuentau deleted the fix_timvx_test branch October 20, 2023 15:29

asmorkalov mentioned this pull request Nov 3, 2023

(5.x) Merge 4.x #24486

Merged

	testing::internal::ParamGenerator< tuple<Backend, Target> > dnnBackendsAndTargetsInt8()
	{
	std::vector< tuple<Backend, Target> > targets;
	targets.push_back(make_tuple(DNN_BACKEND_OPENCV, DNN_TARGET_CPU));
	#ifdef HAVE_TIMVX
	targets.push_back(make_tuple(DNN_BACKEND_TIMVX, DNN_TARGET_NPU));
	#endif
	#ifdef HAVE_INF_ENGINE
	targets.push_back(make_tuple(DNN_BACKEND_INFERENCE_ENGINE_NGRAPH, DNN_TARGET_CPU));
	#endif
	return testing::ValuesIn(targets);
	}

	CV_EXPORTS std::vector< std::pair<Backend, Target> > getAvailableBackends();
	CV_EXPORTS_W std::vector<Target> getAvailableTargets(dnn::Backend be);

	if (withVkCom)
	{
	available = getAvailableTargets(DNN_BACKEND_VKCOM);
	for (std::vector< Target >::const_iterator i = available.begin(); i != available.end(); ++i)
	targets.push_back(make_tuple(DNN_BACKEND_VKCOM, *i));
	}

Uh oh!

Conversation

fengyuentau commented Oct 18, 2023

Pull Request Readiness Checklist

Uh oh!

asmorkalov Oct 18, 2023

Choose a reason for hiding this comment

Uh oh!

fengyuentau Oct 18, 2023

Choose a reason for hiding this comment

Uh oh!

opencv-alalek Oct 18, 2023

Choose a reason for hiding this comment

Uh oh!

fengyuentau Oct 18, 2023

Choose a reason for hiding this comment

Uh oh!

asmorkalov commented Oct 19, 2023

Uh oh!

fengyuentau commented Oct 19, 2023

Uh oh!

fengyuentau commented Oct 19, 2023

Uh oh!

opencv-alalek commented Oct 19, 2023

Uh oh!

opencv-alalek Oct 20, 2023

Choose a reason for hiding this comment

Uh oh!

fengyuentau Oct 20, 2023

Choose a reason for hiding this comment

Uh oh!

fengyuentau Oct 20, 2023

Choose a reason for hiding this comment

Uh oh!

fengyuentau commented Oct 20, 2023

Uh oh!

asmorkalov commented Oct 20, 2023

Uh oh!

asmorkalov left a comment

Choose a reason for hiding this comment

Uh oh!

fengyuentau commented Oct 20, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants