saebyn / munkres-cpp Goto Github PK

Hi,
I'm little confused about the output matrix of munkres.solve(). Why that the numbers of 0 in the result matrix equal to 1 means no match.
thank you.

This bug was found by @kaajo . But unfortunately proposed by @kaajo changes are not complete and correct and require fixes.

Enforce class invariants when DEBUG def'd?

Based on test: MunkresTest.minimize_along_direction_5x5_OverRowsAndColumns_Success result of function minimize_along_direction with over_columns = true is wrong.

Possibly reason (in my opinion):

1. Wrong test case (in particular I have not understood intention of function).
2. Error in algorithm.

Add rules for automatic formatters: astyle, uncrustify, etc...

On the one hand each programmer has personal formating preferences and on the other hand, code on the whole project should looks in the same style.

In any way, in my opinion, it's good feature of mature project.

Extract methods / functions out of the longer steps.

The program generates the following results for the example matrix below:

Input cost matrix =
1.79769e+308 7.33184e+08 9.41561e+08 2.79247e+08
3.06449e+08 1.79769e+308 3.3464e+08 7.06878e+08
9.93296e+08 1.9414e+08 1.79769e+308 1.14174e+08
3.51623e+08 2.48635e+08 7.81242e+08 1.79769e+308
7.02639e+08 8.51663e+08 9.37382e+08 4.96945e+07
7.58851e+08 8.58445e+08 8.7235e+07 5.47076e+08

Munkres assignment =
-1 -1 -1 0
0 -1 -1 -1
-1 -1 -1 -1
-1 0 -1 -1
-1 -1 -1 -1
-1 -1 0 -1
Munkres cost = 9.21566e+08
= 3.06449e8 + 2.48635e8 + 8.7235e7 + 2.79247e8

Alternate assignment =
0 0 0 0
1 0 0 0
0 1 0 0
0 0 0 0
0 0 0 1
0 0 1 0
Alternate cost = 6.37518e+08
= 3.06449e8 + 1.9414e8 + 8.7235e7 + 4.96945e7

Munkres cost = 9.21566e+08
Alternate cost = 6.37518e+08

The input values were generated using

(rand() % 10^9) + 1.0

with DBL_MAX forcibly inserted into the diagonal.

The alternate and Munkres results match when the diagonal values are less than 3.0e24.

Except a lot of benefits it's allow to create tests in easiest way.
For instance using std::initializer_list:
instead of this:

    // Arrange.
    // - + -
    // + - -
    // - - +
    Matrix <double> etalon_matrix (3, 3);
    etalon_matrix (0, 0) = -1.0;
    etalon_matrix (0, 1) = 0.0;
    etalon_matrix (0, 2) = -1.0;
    etalon_matrix (1, 0) = 0.0;
    etalon_matrix (1, 1) = -1.0;
    etalon_matrix (1, 2) = -1.0;
    etalon_matrix (2, 0) = -1.0;
    etalon_matrix (2, 1) = -1.0;
    etalon_matrix (2, 2) = 0.0;
    // 1 0 1
    // 0 1 1
    // 1 1 0
    Matrix <double> test_matrix (3, 3);
    test_matrix (0, 0) = 1.0;
    test_matrix (0, 1) = 0.0;
    test_matrix (0, 2) = 1.0;
    test_matrix (1, 0) = 0.0;
    test_matrix (1, 1) = 1.0;
    test_matrix (1, 2) = 1.0;
    test_matrix (2, 0) = 1.0;
    test_matrix (2, 1) = 1.0;
    test_matrix (2, 2) = 0.0;

write something like this:

    // Arrange.
    Matrix <double> etalon_matrix ({
        {-1.0,  0.0, -1.0},
        { 0.0, -1.0, -1.0},
        {-1.0, -1.0,  0.0}
    });
    Matrix <double> test_matrix ({
        { 1.0,  0.0,  1.0},
        { 0.0,  1.0,  1.0},
        { 1.0,  1.0,  0.0}
    });

Looks like self documented.

I build the project， but failed.

henry@ubuntu:~/CProject/munkres-cpp/build$ make install
[100%] Built target munkres
Install the project...
-- Install configuration: ""
-- Installing: /usr/local/lib/libmunkres.a
CMake Error at cmake_install.cmake:41 (file):
  file INSTALL cannot copy file
  "/home/henry/CProject/munkres-cpp/build/libmunkres.a" to
  "/usr/local/lib/libmunkres.a".


Makefile:85: recipe for target 'install' failed
make: *** [install] Error 1
henry@ubuntu:~/CProject/munkres-cpp/build$ 

Based on test: MunkresTest.solve_3x2_NonObviousSolutionCase001_Success solution result for non square matrix is wrong.

Possibly reason (in my opinion):

1. Wrong test case.
2. Algorithm designed only for square matrix (so special assertion is required).
3. Error in algorithm.

First there's this error:

===>  Testing for munkres-cpp-1.0.0.6
CMake Deprecation Warning at CMakeLists.txt:2 (cmake_minimum_required):
  Compatibility with CMake < 2.8.12 will be removed from a future version of
  CMake.

  Update the VERSION argument <min> value or use a ...<max> suffix to tell
  CMake that the project does not need compatibility with older versions.


-- Configuring done
-- Generating done
-- Build files have been written to: /disk-samsung/freebsd-ports/math/munkres-cpp/work/.build
ninja: no work to do.
ninja: error: 'googletest/googletest/libgtest.a', needed by 'tests/munkrestest', missing and no known rule to make it

Then I applied this patch to use pre-installed googletest:

--- tests/CMakeLists.txt.orig   2021-09-09 05:45:47 UTC
+++ tests/CMakeLists.txt
@@ -3,6 +3,7 @@ find_package (Boost     COMPONENTS      system      RE
 enable_testing ()
 
 # Framework for writing tests. 
+if (FALSE)
 ExternalProject_Add (
     googletest
     GIT_REPOSITORY "https://github.com/google/googletest.git"
@@ -23,6 +24,11 @@ set (GTEST_BOTH_LIBRARIES ${GTEST_LIBRARIES} ${GTEST_M
 
 include_directories (${GTEST_INCLUDE_DIRS})
 
+endif()
+
+find_package(GTest REQUIRED)
+
+
 # Special flags fo generating code coverage.
 set (CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fprofile-arcs -ftest-coverage")
 set (CMAKE_SHARED_LINKER_FLAGS "-fprofile-arcs -ftest-coverage")
@@ -52,7 +58,7 @@ set (
     ${PROJECT_SOURCE_DIR}/tests/adapters/boost_matrixtest.cpp
 )
 add_executable (munkrestest EXCLUDE_FROM_ALL ${MunkresCppLib_SOURCES} ${MunkresCppTest_SOURCES})
-target_link_libraries (munkrestest ${GTEST_BOTH_LIBRARIES} gcov pthread)
+target_link_libraries (munkrestest ${GTest} gcov pthread)
 add_custom_target (tests)
 add_dependencies  (tests munkrestest)
 add_dependencies  (munkrestest googletest)

After this it failed:

/disk-samsung/freebsd-ports/math/munkres-cpp/work/munkres-cpp-1.0.0-6-g61086fc/tests/adapters/std_2d_arraytest.cpp:142:58: error: implicit instantiation of undefined template 'std::__1::array<std::__1::array<double, 3>, 3>'
  std::array <std::array <double, dimension>, dimension> test_array{{
                                                         ^
/usr/include/c++/v1/__tuple:219:64: note: template is declared here
template <class _Tp, size_t _Size> struct _LIBCPP_TEMPLATE_VIS array;
                                                               ^

After adding the missing header it broke again:

/disk-samsung/freebsd-ports/math/munkres-cpp/work/munkres-cpp-1.0.0-6-g61086fc/tests/adapters/std_2d_arraytest.cpp:56:46: error: non-type template argument is not a constant expression
  Std2dArrayAdapter<double,test_array.size(),test_array[0].size()> adapter;
                                             ^~~~~~~~~~~~~~~~~~~~

Hi,
at first, thanks for your work. I'm trying to track two objects through a sequence of frames. I create the matrix using he distances from the object-middle-points from frame t and t+1 (so it is a 2x2 matrix). In the first few frames everything works fine, but suddenly (I debugged it) the solve-method outputs wrong results.
In the following example you can see the matrix I build and the result I get from the solve()-method:
Processing image: ...00080.jpg
11 463.289
452.345 0

0 -1
-1 0

Processing image: ...00081.jpg
12 452.345
440.409 0

0 -1
-1 0

Processing image: ...00082.jpg
428.476 11
12 441.693

0 0
-1 0

When I debug in the last case (image 00082.jpg) the algorithm executes step1() correctly for the first row (STAR is set at matrix(0,1)) in the second row the algorithm comes to the for-loop and in the if-statement it outputs true for STAR == mask_matrix(0,0) -> which is not correct.
for ( size_t nrow = 0 ; nrow < row ; nrow++ )
if ( STAR == mask_matrix(nrow,col) )
That is how the above shown matrix is formed.

It would be super great if you could help me on this, because I can not work correctly with the provided results from the solve()-method.

Thanks in advance.
Best regards
Patricia

sometimes i get the max return:
d2: -0.851823 -0.0848411 0.997337
0.519324 0.752464 0.05258
0.971547 0.304817 -0.311632
dd: -1 -1 0
-1 0 -1
0 -1 -1
pair 0 is :0,2
pair 1 is :1,1
pair 2 is :2,0

sometimes i get the min one:
d2: 0.0511896 -0.0754626 -0.831758
0.541599 0.0293286 -0.483856
0.436718 -0.553582 0.285733
dd: -1 -1 0
0 -1 -1
-1 0 -1
pair 0 is :0,2
pair 1 is :1,0
pair 2 is :2,1

why is that? and how can i fix that?

Method Munkres::pair_in_list always return false.

I have launched tests with option --gtest_repeat=100 and built test coverage report (using lcov). Based report I found that method Munkres::pair_in_list always return false. I have no idea about is this bug or no, but it's looks suspicious. If this method redundant it can be removed, else test case for it must be found.

I have no idea how attach report in issue, so I just attach screenshot.

sometimes i get the max return:
d2: -0.851823 -0.0848411 0.997337
0.519324 0.752464 0.05258
0.971547 0.304817 -0.311632
dd: -1 -1 0
-1 0 -1
0 -1 -1
pair 0 is :0,2
pair 1 is :1,1
pair 2 is :2,0

sometimes i get the min one:
d2: 0.0511896 -0.0754626 -0.831758
0.541599 0.0293286 -0.483856
0.436718 -0.553582 0.285733
dd: -1 -1 0
0 -1 -1
-1 0 -1
pair 0 is :0,2
pair 1 is :1,0
pair 2 is :2,1

why is that? and how can i fix that?

Based on tests: MunkresTest.replace_infinites_4x4Case001_Success, MunkresTest.replace_infinites_4x4Case002_Success, MunkresTest.replace_infinites_4x4Case003_Success and MunkresTest.replace_infinites_4x4Case004_Success result of function replace_infinites is wrong.

It is looks like real bug (when determined min it not checked on equality to infinity, so max became infinity).

Just want to note, that whenever you write code (you usually see this kind of thing in various tutorials) like this

    m_matrix = new T*[rows]; // rows
    for ( size_t i = 0 ; i < rows ; i++ ) {
      m_matrix[i] = new T[columns]; // columns
    }

you have a memory leak, because any of the new T[columns] can throw an exception.
Standard way to fix that is to either use std::unique_ptr or std::vector. But even better thing to do here is to use one std::vector of size columns * rows, because that will improve cache locality.

Support other matrix data type classes from other libs (boost)?

UPD
Implemented:

• std::array (C++11);
• raw (C-style) array;
• boost::numeric::ublas::matrix;
• std::vector <std::vector <double> >.

Replace cxxtest with http://code.google.com/p/googletest/ .

During migration from GNU Makefile to CMake "install" target was accidentally lost.

solve before
Matrix:
0, 0, 0,
0, 0, 0,
0, 0, 0,
0, 0, 0,
0, 0, 0,
0, 0, 0,
0, 0, 0,
0, 0, 0,
0, 0, 0,
0, 0, 0,
0, 0, 0,
0, 0, 0,
0, 0, 0,
0, 0, 0,
0, 0, 0,
0, 0, 0,
0, 0, 0,
0, 0, 0,
0, 0, 0,
0, 0, 0,
0, 0, 0,
0, 0, 0,
0, 0, 0,
0, 0, 0,
0, 0, 0,
0, 0, 0,
0, 0, 0,
0, 0, 0,
0, 0, 0,
0, 0, 0,
0.868258, 0.86991,0.869284,
0,0.813252,0.812353,
0, 0, 0,
0, 0, 0,
0, 0, 0,
0, 0, 0,
0, 0, 0,
0, 0, 0,
0, 0, 0,
0, 0, 0,
0, 0, 0,
0, 0, 0,
0, 0, 0,
0, 0, 0,
0, 0, 0,
0, 0, 0,
0, 0, 0,

solve after
Matrix:
0, -1, -1,
-1, 0, -1,
-1, -1, 0,
-1, -1, -1,
-1, -1, -1,
-1, -1, -1,
-1, -1, -1,
-1, -1, -1,
-1, -1, -1,
-1, -1, -1,
-1, -1, -1,
-1, -1, -1,
-1, -1, -1,
-1, -1, -1,
-1, -1, -1,
-1, -1, -1,
-1, -1, -1,
-1, -1, -1,
-1, -1, -1,
-1, -1, -1,
-1, -1, -1,
-1, -1, -1,
-1, -1, -1,
-1, -1, -1,
-1, -1, -1,
-1, -1, -1,
-1, -1, -1,
-1, -1, -1,
-1, -1, -1,
-1, -1, -1,
-1, -1, -1,
-1, -1, -1,
-1, -1, -1,
-1, -1, -1,
-1, -1, -1,
-1, -1, -1,
-1, -1, -1,
-1, -1, -1,
-1, -1, -1,
-1, -1, -1,
-1, -1, -1,
-1, -1, -1,
-1, -1, -1,
-1, -1, -1,
-1, -1, -1,
-1, -1, -1,
-1, -1, -1,