Skip to content

cpp DAAL implementation - wrapper for raw pointer input#527

Open
stem-lab-tech wants to merge 11 commits intomasterfrom
daal_cpp
Open

cpp DAAL implementation - wrapper for raw pointer input#527
stem-lab-tech wants to merge 11 commits intomasterfrom
daal_cpp

Conversation

@stem-lab-tech
Copy link
Copy Markdown
Contributor

cpp part of DAAL implementation

@stem-lab-tech stem-lab-tech self-assigned this Mar 28, 2018
@mdymczyk
Copy link
Copy Markdown
Contributor

mdymczyk commented Apr 6, 2018

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 mdymczyk self-requested a review April 6, 2018 04:31
@mdymczyk mdymczyk added this to the 0.2.1 milestone Apr 6, 2018
@stem-lab-tech
Copy link
Copy Markdown
Contributor Author

@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.

@mdymczyk
Copy link
Copy Markdown
Contributor

mdymczyk commented Apr 11, 2018

@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.

@stem-lab-tech
Copy link
Copy Markdown
Contributor Author

@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.

@mdymczyk mdymczyk modified the milestones: 0.2.1, 0.2.2 Apr 30, 2018
@pseudotensor pseudotensor modified the milestones: 0.2.2, 0.4.0 Sep 28, 2018
@sh1ng sh1ng modified the milestones: 0.3.2, 0.4.0 Jul 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants