mlcpp / matrix Goto Github PK

GitHub Socialify should be added to README to make it more attractive and make the project appealing to a vast audience.

Currently, we can only initialize a Matrix object using <std::vector<std::vector<double>> or <std::vector<std::vector<std::string>>.
So, if I want to create a Matrix with 1 row and 1 column I need to create a 2-D vector object and then use the init() method which seems a bit redundant.

Describe the solution you'd like

Overload init() method such that we can use a double, std::string, std::vector<double> and std::vector<std::string> to initialize a Matrix object.

Expectation

After completion, init(2) should give me a Matrix object with 1 element. Similarly, for 1-D vectors.

The parameter description of read_csv() is not accurate in README.md

Job of std:string parameter should be "Path of the" instead of "File Name of the".

Note: You can contact @ashok-arora if you face any problem while solving this issue.

It is impossible to work with const Matrix objects as none of the methods are declared const.

Description

Using a const Matrix object I can't even call basic methods like row_length(), col_length(), print(), etc. as none of these are declared const` even though they are not changing the object value.

Expected Behavior

row_length(), col_length() and other methods not changing the object should run when called using a const Matrix object.

Actual Behavior

row_length(), col_length() and other methods not changing the object give error when when called using a const Matrix object.

Possible Fix

Add const keyword to methods which will not change the object value.

Note: You can contact @aniketsharma00411 if you face any problem while solving this issue.

Examples are created to make it easier for beginners to use the library. But the size of the Boston dataset makes them complext and hampers their usability.

Thus, a smaller dataset (like the one in the tests/ directory) should be used to improve them.

Add the smaller dataset in the examples/ directory only.
Recompile the examples using make and make sure that they are working properly.

The naming scheme and use of Matrix methods should be similar to that of Python NumPy. Accordingly, read_csv method should be changed to genfromtxt.

Steps to follow:

1. Update the read_csv method in matrix_read.hpp to take delimiter as an argument.
2. Rename read_csv to genfromtxt and add it in matrix_operations.hpp
3. Delete matrix_read.hpp and update Matrix.hpp to remove the statement which includes matrix_read.hpp
4. Update the README.md accordingly
5. Rename examples/read_csv.cpp to examples/genfromtxt.cpp and update the example accordingly.
6. In examples/CMakeLists.txt, update the line add_executable(read_csv read_csv.cpp) to add_executable(genfromtxt genfromtxt.cpp)
7. Rename benchmark/BM_read_csv.cpp to benchmark/BM_genfromtxt.cpp and update the benchmark to use new method name.
8. Delete benchmark/linux_binaries/BM_read_csv and generate new binary using g++ benchmark/BM_genfromtxt.cpp -std=c++11 -I./include/ -isystem ./lib/benchmark/include -L./lib/benchmark/build/src -lbenchmark -lpthread -o ./benchmark/linux_binaries/BM_genfromtxt
9. Rename BM_read_csv in benchmark/BM_all.cpp to BM_genfromtxt update the method call according to the new name. Also, position the method according to lexicographical order.
10. Recompile benchmark/BM_all.cpp using g++ benchmark/BM_all.cpp -std=c++11 -I./include/ -isystem ./lib/benchmark/include -L./lib/benchmark/build/src -lbenchmark -lpthread -o ./benchmark/linux_binaries/BM_all

Note: All the above steps should be a part of one commit with the message "Renames read_csv to genfromtxt".

Note: You can contact @ashok-arora if you face any problem while solving this issue.

To improve readability method declarations should be in a .hpp file and definitions should be in a .cpp file which is not the case currently.

Note: Do not change file names.

To provide an easier way to view API documentation, the function comments should be converted to Doxygen comments.

The naming scheme of Matrix methods should be similar to that used in Python NumPy. Accordingly, del method should be renamed to delete.

Steps to follow:

1. Rename the method in matrix_operations.hpp
2. Update the README.md accordingly
3. Update examples/delete.cpp to use the new method name.
4. Rename benchmark/BM_del.cpp to benchmark/BM_delete.cpp and update the benchmark to use new method name.
5. Delete benchmark/linux_binaries/BM_del and generate new binary using g++ benchmark/BM_delete.cpp -std=c++11 -I./include/ -isystem ./lib/benchmark/include -L./lib/benchmark/build/src -lbenchmark -lpthread -o ./benchmark/linux_binaries/BM_delete
6. Rename BM_del_row and BM_del_column methods in benchmark/BM_all.cpp to BM_delete_row and BM_delete_column respectively and update the method call according to the new name.
7. Recompile bencmark/BM_all.cpp using g++ benchmark/BM_all.cpp -std=c++11 -I./include/ -isystem ./lib/benchmark/include -L./lib/benchmark/build/src -lbenchmark -lpthread -o ./benchmark/linux_binaries/BM_all

Note: All the above steps should be a part of one commit with the message "Renames del to delete".

Is your feature request related to a problem? Please describe.

Compiling manually is a time-consuming task.

Describe the solution you'd like

cmake can be used to compile tests/tests.cpp

Describe alternatives you've considered

Currently, I am using the below command to compile and run

cd tests
g++ tests.cpp -I../include -lgtest -lpthread -o ../build/tests/tests && ../build/tests/tests

The CMake gives the error

-- The CXX compiler identification is unknown
CMake Error at CMakeLists.txt:2 (project):
  The CMAKE_CXX_COMPILER:

    cl

  is not a full path and was not found in the PATH.

  To use the NMake generator with Visual C++, cmake must be run from a shell
  that can use the compiler cl from the command line.  This environment is
  unable to invoke the cl compiler.  To fix this problem, run cmake from the
  Visual Studio Command Prompt (vcvarsall.bat).

  Tell CMake where to find the compiler by setting either the environment
  variable "CXX" or the CMake cache entry CMAKE_CXX_COMPILER to the full path
  to the compiler, or to the compiler name if it is in the PATH.


-- Configuring incomplete, errors occurred!
See also "C:/Users/Harshit_Singh/Documents/my doc/linear_algebra/Matrix/build/CMakeFiles/CMakeOutput.log".
See also "C:/Users/Harshit_Singh/Documents/my doc/linear_algebra/Matrix/build/CMakeFiles/CMakeError.log".

but the file compiles when the file is compiled using the following code:

 g++ examples/abs.cpp -std=c++11 -I./include/ -o ./build/abs

The current build system does not compile the benchmarking files present in the benchmark/ directory.

The directory structure will be

/Matrix
  /benchmark
    /linux_binaries
    /macos_binaries
    /windows_executables
    ...
  ...

When in Matrix/benchmark/ the command used to compile will be

g++ <file_name>.cpp -std=c++11 -I../include/ -isystem ../lib/benchmark/include -L../lib/benchmark/build/src -lbenchmark -lpthread -o ./linux_binaries/<file_name>

Note: You can contact @aniketsharma00411 if you face any problem while solving this issue.

All edge cases are not covered by current unit tests thus more unit tests should be added to cover those.

All methods currently have atleast one unit test for them and all tests for a method should be present in the same file.

Note: This issue will not be assigned to anyone but anyone can work on it. Just create a pull request mentioning the issue and what edge cases you covered which were not previously checked. This issue will not be closed.

Currently, the required CMake version is 3.19.0 which is not yet available for all OSs.

The required version is CMakeLists.txt should be the oldest which will work without any issue.

The naming scheme of Matrix methods should be similar to that used in Python NumPy. Accordingly, concat method should be renamed to concatenate.

Steps to follow:

1. Rename the method in matrix_operations.hpp
2. Update the README.md accordingly
3. Rename examples/concatenation.cpp to examples/concatenate.cpp and update the example accordingly.
4. Rename benchmark/BM_concat.cpp to benchmark/BM_concatenate.cpp and update the benchmark to use new method name.
5. Delete benchmark/linux_binaries/BM_concat and generate new binary using g++ benchmark/BM_concatenate.cpp -std=c++11 -I./include/ -isystem ./lib/benchmark/include -L./lib/benchmark/build/src -lbenchmark -lpthread -o ./benchmark/linux_binaries/BM_concatenate
6. Rename BM_concat_row and BM_concat_column methods in benchmark/BM_all.cpp to BM_concatenate_row and BM_concatenate_column respectively and update the method call according to the new name.
7. Recompile benchmark/BM_all.cpp using g++ benchmark/BM_all.cpp -std=c++11 -I./include/ -isystem ./lib/benchmark/include -L./lib/benchmark/build/src -lbenchmark -lpthread -o ./benchmark/linux_binaries/BM_all

Note: All the above steps should be a part of one commit with the message "Renames concat to concatenate".

make bechmarks and make tests results in an error when compiling in Windows. make examples works as expected.

Expected Behavior

Bechmarks and Tests binaries should have been created.

Actual Behavior

Error

Possible Fix

New version of Google Benchmark and googletest causing this error.

Is your feature request related to a problem? Please describe.

After cloning the repo and following the installation procedure, make benchmarks gives error that google benchmark is not installed.

Describe the solution you'd like

make, make all and make benchmarks should install google benchmark if not found. Similarly, make, make all and make tests should install googletest in not found in lib/ directory.

Describe alternatives you've considered

I need to install google benchmark and googletest everytime when working on a fresh clone from their respective repo.

Integrate Google Test into this repository.