[Bug]: sum_rows() and sum_cols are reversed

What Operating System(s) are you seeing this problem on?

Linux (x86-64)

dlib version

19.24

Python version

No response

Compiler

11.4.0

Expected Behavior

sum_rows() sums the entries of a row of a matrix.

sum_cols() sums the entries of a column of a matrix.

Current Behavior

sum_rows() sums the entries of column of a matrix.
This is also reflected by the documentation, ie. sum_rows() contains sum(colm(m,i)), s.

dlib/dlib/matrix/matrix_utilities_abstract.h

Line 1606 in ca7fd9d

- M(i) == sum(colm(m,i))

sum_cols() sums the entries of row of a matrix.
This is also reflected by the documentation, ie. sum_cols() contains sum(rowm(m,i)), s.

dlib/dlib/matrix/matrix_utilities_abstract.h

Line 1624 in ca7fd9d

- M(i) == sum(rowm(m,i))

Example:

#include <dlib/matrix.h>

int main() {
    dlib::matrix<double, 3, 3> M = {1, 2, 3, 4, 5, 6, 7, 8, 9};
    std::cout << "M:\n" << M << std::endl;

    dlib::matrix<double> sum_rows = dlib::sum_rows(M);
    dlib::matrix<double> sum_cols = dlib::sum_cols(M);

    std::cout << "Sum of Rows:\n" << sum_rows << std::endl;
    std::cout << "Sum of Cols:\n" << sum_cols << std::endl;
}

outputs

M:
1 2 3 
4 5 6 
7 8 9 

Sum of Rows:
12 15 18 

Sum of Cols:
 6 
15 
24

Steps to Reproduce

Source Code

Run the following code to see the differences.

#include <dlib/matrix.h>

int main() {
    dlib::matrix<double, 3, 3> M = {1, 2, 3, 4, 5, 6, 7, 8, 9};
    std::cout << "M:\n" << M << std::endl;

    dlib::matrix<double> sum_rows = dlib::sum_rows(M);
    dlib::matrix<double> sum_cols = dlib::sum_cols(M);

    std::cout << "Sum of Rows:\n" << sum_rows << std::endl;
    std::cout << "Sum of Cols:\n" << sum_cols << std::endl;
}

CMakeLists.txt

cmake_minimum_required(VERSION 2.8.12)
project(examples)

set(CMAKE_EXPORT_COMPILE_COMMANDS ON)
SET(DEBUG OFF)
if(DEBUG)
    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -g")
endif()

include(FetchContent)
FetchContent_Declare(dlib
 GIT_REPOSITORY https://github.com/davisking/dlib.git
 GIT_TAG        v19.24
)
FetchContent_MakeAvailable(dlib)

include_directories(/data/upas2-resources/inst/include)

add_executable(sum_rows_cols sum_rows_cols.cpp)

ADD_CUSTOM_TARGET(
    sum
    COMMAND ${CMAKE_BINARY_DIR}/sum_rows_cols     
    DEPENDS sum_rows_cols 
    WORKING_DIRECTORY ${CMAKE_BINARY_DIR}
    COMMENT "Calculate Sums"
)

Anything else?

No response

After reading the documentation, it seems to me like it's working as intended.

I read

    const matrix_exp sum_rows (
        const matrix_exp& m
    );
    /*!
        requires
            - m.size() > 0
        ensures
            - returns a row matrix that contains the sum of all the rows in m. 
            - returns a matrix M such that
                - M::type == the same type that was in m
                - M.nr() == 1
                - M.nc() == m.nc()
                - for all valid i:
                    - M(i) == sum(colm(m,i)) 
    !*/

especially sum(colm(m, i)) as a sum over column i of matrix m, although the function is called sum_rows and the first sentence says that it "returns a row matrix that contains the sum of all the rows in m." I can't exclude mistakes on my side. I'm glad for clarification.

Source:

dlib/dlib/matrix/matrix_utilities_abstract.h

Line 1593 in ca7fd9d

const matrix_exp sum_rows (

Yep, it's doing what it's supposed to do :)

Then I'm puzzled. When sum_rows() and sum_cols() work appropriately, am I reading the output of my example program incorrectly?

This?

M:
1 2 3 
4 5 6 
7 8 9 

Sum of Rows:
12 15 18 

Sum of Cols:
 6 
15 
24

That's the right output. sum_rows() adds all the row vectors together, which gives the result you have there from your program.

Maybe I can break it for you:

M has three rows:

1 2 3
4 5 6
7 8 9

In order to sum the rows, we have to add the first row to the second row and then to the third row.

That means adding
1+4+7, 2+5+8, 3+6+9

If you take the first row (1 2 3) and add the elements together, you are not adding the rows, you are adding the columns. If you write the loops to do that, you'll see that when you do the addition, the row index stays constant, and the column index changes: you are adding the columns.

I am very glad how kind you respond to my issue! (I also made ..let's say, "other experiences".) I feel very sorry, that I flagged my issue directly as "Bug". I should have started a discussion or opened a normal issue.

Although I closed the issue, I want to give some feedback—you can freely decide, what you will do with it!

For me "sum of all the rows" still sounds more like I thought in the first place (but I am obviously biased). Hence, the documentation, ie. sum(colm(m, i)), looked contradictory. I would advocate for a less ambiguous description. In case you agree, I can try my best and submit a PR.

No worries.

What if it said "sum of all the row vectors", that clearer?

Yes, definitely! 🚀

(In my opinion, sum_row_vectors() additionally as function name would be even better but I guess this is not feasible due to backwards compatibility, ie. deleting/changing the name sum_rows() is a big nono. 😄)

Yeah, updated the docs. Can't rename the functions though, too much code and people are using the current names :D

Nice!

... As mentioned, I can fully understand a renaming is out of scope.. but we all have hopes and wishes! :D

To conclude: Thank you for the nice conversation on my fake bug! :)

My pleasure :D