Removed fill method

[fiza11]

10

Removed fill method from both Matrix and Vector classes and shifted to operations module.

[czgdp1807]

The tests are failing. Some fixes are needed.

[czgdp1807]

@fiza11 Why have you closed this PR?

[fiza11]

@czgdp1807 I'm sorry. I closed it by mistake.

[czgdp1807]

@fiza11 You can test your changes by executing the following series of commands, provided you are under project root i.e., adaboost

1. cd ../
2. mkdir build-adaboost
3. cd build-adaboost
4. cmake -DBUILD_TESTS=ON -DBUILD_CUDA=ON ../adaboost
5. make -j5
6. cd bin
7. ./test_core
8. ./test_cuda

I hope you have a GPU on your system. Keep on doing the changes until tests pass locally. If you don't have GOOGLE TEST, you can install it by executing the commands below from your home directory,

1. wget https://github.com/google/googletest/archive/release-1.10.0.tar.gz
2. tar -zxvf release-1.10.0.tar.gz
3. ls -a
4. mkdir googletest-build
5. cd googletest-build
6. sudo ../cmake-3.10.2-Linux-x86_64/bin/cmake ../googletest-release-1.10.0
7. sudo make -j5
8. sudo make install

[czgdp1807]

Please resolve the conflicts by merging master into your branch. Probably this happened due to change in directory structure.

[czgdp1807]

I have restored the deleted files in your branch.

1. The following lines should be under cuda/core/operations.hpp and the respective implementation should be under, cuda/core/operations_impl.cpp. Currently you have placed both CPU and CUDA implementations under core/operations_impl.cpp. https://github.com/fiza11/adaboost/blob/4ed50dd141feeb34e90289eec7f9fb370e60e360/adaboost/core/operations.hpp#L55-L56

2. Changes should be made at places where call to fill has been made such as in the line below, https://github.com/fiza11/adaboost/blob/4ed50dd141feeb34e90289eec7f9fb370e60e360/adaboost/cuda/core/cuda_data_structures_impl.hpp#L71

You did the changes in core module but forgot to do the same in cuda/core.

[czgdp1807]

[ 27%] Built target adaboost_cuda
[ 50%] Built target adaboost_core
[ 61%] Built target test_core
[ 77%] Built target adaboost_cuda_wrappers
[ 88%] Built target adaboost_utils
[ 94%] Linking CXX executable ../bin/test_cuda
CMakeFiles/test_cuda.dir/tests/test_cuda.cpp.o: In function `Cuda_VectorGPU_Test::TestBody()':
test_cuda.cpp:(.text+0x72): undefined reference to `void adaboost::cuda::core::fill<float>(float, adaboost::cuda::core::VectorGPU<float> const&, unsigned int)'
test_cuda.cpp:(.text+0x8e): undefined reference to `void adaboost::cuda::core::fill<float>(float, adaboost::cuda::core::VectorGPU<float> const&, unsigned int)'
test_cuda.cpp:(.text+0xc9): undefined reference to `void adaboost::cuda::core::product_gpu<float>(adaboost::cuda::core::VectorGPU<float> const&, adaboost::cuda::core::VectorGPU<float> const&, float&, unsigned int)'
test_cuda.cpp:(.text+0x4af): undefined reference to `void adaboost::cuda::core::product_gpu<float>(adaboost::cuda::core::VectorGPU<float> const&, adaboost::cuda::core::VectorGPU<float> const&, float&, unsigned int)'
CMakeFiles/test_cuda.dir/tests/test_cuda.cpp.o: In function `Cuda_MatrixGPU_Test::TestBody()':
test_cuda.cpp:(.text+0x986): undefined reference to `void adaboost::cuda::core::fill<float>(float, adaboost::cuda::core::MatrixGPU<float> const&, unsigned int, unsigned int)'
test_cuda.cpp:(.text+0x9a4): undefined reference to `void adaboost::cuda::core::fill<float>(float, adaboost::cuda::core::MatrixGPU<float> const&, unsigned int, unsigned int)'
test_cuda.cpp:(.text+0xa0d): undefined reference to `void adaboost::cuda::core::multiply_gpu<float>(adaboost::cuda::core::MatrixGPU<float> const&, adaboost::cuda::core::MatrixGPU<float> const&, adaboost::cuda::core::MatrixGPU<float>&)'
test_cuda.cpp:(.text+0xc0e): undefined reference to `void adaboost::cuda::core::multiply_gpu<float>(adaboost::cuda::core::MatrixGPU<float> const&, adaboost::cuda::core::MatrixGPU<float> const&, adaboost::cuda::core::MatrixGPU<float>&)'
collect2: error: ld returned 1 exit status
adaboost/CMakeFiles/test_cuda.dir/build.make:96: recipe for target 'bin/test_cuda' failed
make[2]: *** [bin/test_cuda] Error 1
CMakeFiles/Makefile2:281: recipe for target 'adaboost/CMakeFiles/test_cuda.dir/all' failed
make[1]: *** [adaboost/CMakeFiles/test_cuda.dir/all] Error 2
Makefile:129: recipe for target 'all' failed
make: *** [all] Error 2

Fix required.

[Tanvi141]

@czgdp1807, changes have been made, I just noticed that ./test_cuda is giving errors sometimes. Posting the error here. Any idea why I am getting this error? It only comes sometimes, not everytime. I am also trying to look into it.

 tanvi@ubuntu  ~/OpenSource/fiza/build-adaboost/bin  ./test_cuda
[==========] Running 2 tests from 1 test case.
[----------] Global test environment set-up.
[----------] 2 tests from Cuda
[ RUN      ] Cuda.VectorGPU
[       OK ] Cuda.VectorGPU (53 ms)
[ RUN      ] Cuda.MatrixGPU
/home/tanvi/OpenSource/fiza/adaboost/./adaboost/tests/test_cuda_data_structures.hpp:63: Failure
      Expected: 60.0
      Which is: 60
To be equal to: result1.at(i, j)
      Which is: 40
[  FAILED  ] Cuda.MatrixGPU (1 ms)
[----------] 2 tests from Cuda (54 ms total)

[----------] Global test environment tear-down
[==========] 2 tests from 1 test case ran. (54 ms total)
[  PASSED  ] 1 test.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] Cuda.MatrixGPU

EDIT: I just tried running ./test_cuda on the original repository (on which latest commit is my updation of directory structure) and turns out I am also getting ./test_cuda failing with the same above message. Here also I am getting the message only sometimes, most of the time is shows all tests passed. Could this be an issue with my GPU?

[czgdp1807]

I have also experienced similar kind of non-deterministic results. You can just paste the results whenever they pass. Please open a new issue for the random failures.

[Tanvi141]

@czgdp1807 I have a doubt, what is the motivation behind replacing original for loops with std::fill? (from your suggested changes). Does it have any performance optimization?

[czgdp1807]

@czgdp1807 I have a doubt, what is the motivation behind replacing original for loops with std::fill? (from your suggested changes). Does it have any performance optimization?

https://comp.lang.cpp.moderated.narkive.com/x3loglLJ/std-memset-std-fill-hand-written-for-loop

P.S. Please post the passing CUDA tests so that we can merge this PR. Try running CUDA tests multiple times, as soon as they pass copy and past the reports here.

[Tanvi141]

[==========] Running 2 tests from 1 test suite.
[----------] Global test environment set-up.
[----------] 2 tests from Cuda
[ RUN      ] Cuda.VectorGPU
[       OK ] Cuda.VectorGPU (350 ms)
[ RUN      ] Cuda.MatrixGPU
[       OK ] Cuda.MatrixGPU (0 ms)
[----------] 2 tests from Cuda (350 ms total)

[----------] Global test environment tear-down
[==========] 2 tests from 1 test suite ran. (350 ms total)
[  PASSED  ] 2 tests.