Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cpp DAAL implementation - wrapper for raw pointer input #527

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

stjoern
Copy link
Contributor

@stjoern stjoern commented Mar 28, 2018

cpp part of DAAL implementation

@stjoern stjoern self-assigned this Mar 28, 2018
@mdymczyk
Copy link
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
@stjoern
Copy link
Contributor Author

stjoern commented Apr 8, 2018

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

@stjoern
Copy link
Contributor Author

stjoern commented Apr 23, 2018

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

None yet

4 participants