h2o4gpu icon indicating copy to clipboard operation
h2o4gpu copied to clipboard

cpp DAAL implementation - wrapper for raw pointer input

Open stem-lab-tech opened this issue 7 years ago • 4 comments

cpp part of DAAL implementation

stem-lab-tech avatar Mar 28 '18 23:03 stem-lab-tech

Looks good @stjoern, think it's ready for a merge or are you still working on something? I will merge my CMake/SWIG stuff once I get an approval today/tomorrow and we can look into merging this if you think it's merge-ready.

mdymczyk avatar Apr 06 '18 04:04 mdymczyk

@mdymczyk yes, it's ready to merge, you one can now build dynamic librarylibh2o4gpu_daal.so, but currently only locally, since we do not have consumer in Java and in python is already there. I hope SWIG is able to consume full c++ containers, no only C raw pointers, for all the cases I have added C_API interface, tested it with my local environment settings and it works.

stem-lab-tech avatar Apr 08 '18 20:04 stem-lab-tech

@stjoern ok a few things, sorry for the wall of text.

  1. I moved the source files from src/cpu to src/daal - the reason being we want to:
  • put in cpu only the CPU code that we always ship
  • in gpu only the CUDA code
  • in common shared code between CPU and GPU
  • in other new folders - code that can be built as a separate .o file and shipped conditionally

Now it makes sense to me to put DAAL code into a separate folder and generate it into a separate .o file and conditionally link it with out CPU .so like I did now in the CMakeLists.txt, what do you think?

It was failing to compile before because of how the current CMake is structured. It was grabbing recursively all the cpp/h files in src/cpu so including DAAL stuff and was trying to build our library but wasn't including the necessary DAAL headers etc. (which you do in the daal/Makefile).

  1. I removed the DAAL Makefile and instead put the DAAL source compilation in the CMake - we want to use as little Makefiles as possible nowadays.

  2. I haven't fully tested this as I cannot seem to get DAAL to install fully, I ran the install scripts and they seem to work but when I run ldconfig -p | grep daal I get a blank return. Should I do anything more?

  3. Last but not least - we will need to test this somehow, from what I see only alldeps_install-cpuonly installs DAAL and we're not using this in Jenkins so we're not really testing DAAL currently unless I'm missing something? I tried checking the Jenkins machines but they don't seem to have it installed?

I also couldn't find any logs in previous master builds of DAAL tests (the python tests like for example [gw0] [ 6%] PASSED tests_open/glm/test_glm_simple.py::test_glm_simple_gpu_fold1_quick_0 but for DAAL). I tried running them locally and they all pass but I'm getting (when running with verbose) logs from our CUDA tsvd backend. I think we might need a bit more logging when verbose is set high to make sure which backend is being run.

  1. Ah one more minor thing - we should remove the pyenv reference in the install scripts, not everyone is using pyenv.

mdymczyk avatar Apr 11 '18 12:04 mdymczyk

@mdymczyk Mateusz, I've prepared working library of DAAL in c++ with raw interface in pure C. I am not good with SWIG, but I guess we don't need pure C_api interface, since SWIG should be able to use straight C++ containers. However to demonstrate, the library is working I 've added python script with test examples how you could use it with java interface.

/interface_py/h2o4gpu/daal_c_interface are python files with particular examples. The reason why it's not currently called from jenkins is that, that we don't need to call python c_api interface, since we reach the same effect with PyDAAL interface.

In folder src/daal/ is Makefile (for building libh2o4gpu_daal.so used with python files). In the main Makefile is rule:

install_daal_x86_64:, this should download needed headers and other libraries, install on your system and make libh2o4gpu_daal.so build. The python files serve only as an example how to use it with raw c.

stem-lab-tech avatar Apr 23 '18 11:04 stem-lab-tech