Linalg - Vector Class #3277

OXPHOS · 2016-06-09T21:19:31Z

No description provided.

lambday · 2016-06-09T21:38:40Z

src/shogun/mathematics/linalg/SGLinalg.cpp

@@ -33,6 +33,8 @@

 #include <shogun/mathematics/linalg/SGLinalg.h>

+#include<iostream>


This should not be required :)

vigsterkr · 2016-06-10T06:37:58Z

src/shogun/mathematics/linalg/BaseVector.h

@@ -54,7 +54,7 @@ struct BaseVector
 	}

 	/** Data Storage */
-	virtual bool onGPU() = 0 ;
+	virtual bool onGPU() const = 0 ;


forgive my ignorance but if we have CPUVector and GPUVector then why do we need onGPU function. since the class could identify the storage or?

well, this was decided because dynamic cast is thought to be slower compared to one virtual call + static cast. But I see what you mean. We probably won't need to have two different classes, only one class should suffice.

@OXPHOS what was the issue again with putting both these thing inside a single class? I think it is doable.

well instead of dynamic casting see the SGObject get_name virtual function. that's a more general way to identify the type of the object without dynamic casting.

Okay thanks. But maybe we won't have to do the casting, if we can do what you discussed earlier in the gist @OXPHOS made - have it in the same class and call onGPU method. I can't seem to remember what was stopping us from doing that.

yeah that would be more ideal indeed.

OXPHOS · 2016-06-13T06:51:04Z

ref #3278
The vector class implementation based on my understand.

So the user will convert SGVector -> LinalgVector for both CPU and GPU vector as the first step.
Vector can be transferred to GPU if user explicitly calls LinalgVector.transfer_to_GPU().

I'll check #3278 'comments for more details tomorrow as it is kinda late now.
@karlnapf @lambday @vigsterkr

OXPHOS · 2016-06-13T06:54:34Z

src/shogun/mathematics/linalg/GPUVectorImpl.h

+
+#ifdef HAVE_CXX11
+#ifdef HAVE_VIENNACL
+#include <viennacl/vector.hpp>


This class should be able to transfer vectors from CPU to GPU via multiple GPU libraries (and use whatever GPU library the user assigns) in the future

lambday · 2016-06-14T09:32:43Z

src/shogun/mathematics/linalg/linalgVector.h

+private:
+	alignas(CPU_CACHE_LINE_SIZE) bool m_onGPU;
+	alignas(CPU_CACHE_LINE_SIZE) index_t m_len;  // same logic
+	alignas(CPU_CACHE_LINE_SIZE) T* m_data;      // non-owning ptr, referring to the SGVector.vector


The comment // non-owning ptr, referring to the SGVector.vector is no longer valid, since this is now a deep copy. Can we please have shared-ptr for m_data here instead?

lambday · 2016-06-14T09:48:48Z

Hi @OXPHOS !

Looks good. Please check my comments. Also, please squash all commits together. We should be able to merge this soon!

lambday · 2016-06-14T09:50:09Z

Also, please add unit-tests to all the methods you added. Specially transferToCPU, transferToGPU, cast operators, etc etc. Make sure everything works with valgrind as well :)

OXPHOS · 2016-06-14T20:25:05Z

There's still a problem - say we have both CPU and GPU storage for the same vector. Now every time when we re extracting the Vector, we update the Vector based on GPU information. However, if someone transferred the vector to GPU, but have the elementwise.product done on CPU, GPU is actually behind CPU. In this case, current SGVector<T>() -> transfertoCPU() doesn't make sense at all.

I can think of 1). If onGPU()==true and the calculation is done on CPU, we synchronize GPU vector everytime. This will be slow. 2). Ask the user to transfer/synchronize GPU/CPU data before using Vector instance to initialize SGVector

lambday · 2016-06-14T22:21:11Z

Aren't we performing the operations on GPU if onGPU returns true by design? If you check the dot method that you implemented (other methods will be implemented in a similar fashion), then there won't be any case when GPU data is present but the operation is performed on CPU. Does it solve the issue?

lambday · 2016-06-14T22:50:50Z

src/shogun/mathematics/linalg/Vector.cpp

+Vector<T>::Vector(SGVector<T> const &vector)
+{
+	init();
+	m_data = std::shared_ptr<T>(reinterpret_cast<T*>(SG_MALLOC(aligned_t, vector.vlen)), free);


Please use SG_FREE instead of free since you have used SG_MALLOC :)

karlnapf · 2016-06-15T16:04:30Z

src/shogun/mathematics/linalg/GPUVectorImpl.cpp

+}
+
+template <class T>
+viennacl::const_entry_proxy<T> Vector<T>::GPUVectorImpl::operator[](index_t index) const


This is a dangerous operations, as expensive. Remove.
For unit tests, we can just transfer the full structure back to main memory.
There is no need to access single elements of gpu structures, just do linalg operations on them

vigsterkr · 2016-06-15T20:30:32Z

http://www.stack.nl/~dimitri/doxygen/manual/docblocks.html

lambday reviewed Jun 9, 2016
View reviewed changes

OXPHOS force-pushed the linalg branch from e4f563a to 65f6230 Compare June 10, 2016 02:40

vigsterkr reviewed Jun 10, 2016
View reviewed changes

merge vector class

f826ba6

OXPHOS force-pushed the linalg branch from 27594b6 to f826ba6 Compare June 13, 2016 06:46

OXPHOS changed the title ~~Linalg - Vector Convert Method~~ Linalg - Vector Class Jun 13, 2016

OXPHOS reviewed Jun 13, 2016
View reviewed changes

mem aglin

a7d1e42

lambday reviewed Jun 14, 2016
View reviewed changes

update - /shared_ptr/gpu copy/test cases

b954a77

lambday reviewed Jun 14, 2016
View reviewed changes

karlnapf reviewed Jun 15, 2016
View reviewed changes

OXPHOS added 2 commits June 15, 2016 21:05

shallow copy / move around transfer method

bde4bd0

ref_count

27f3341

OXPHOS closed this Jun 22, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Linalg - Vector Class #3277

Linalg - Vector Class #3277

OXPHOS commented Jun 9, 2016 •

edited

lambday Jun 9, 2016

vigsterkr Jun 10, 2016

lambday Jun 10, 2016

lambday Jun 10, 2016

vigsterkr Jun 10, 2016

lambday Jun 10, 2016

vigsterkr Jun 10, 2016

OXPHOS commented Jun 13, 2016 •

edited

OXPHOS Jun 13, 2016 •

edited

lambday Jun 14, 2016

lambday commented Jun 14, 2016

lambday commented Jun 14, 2016

OXPHOS commented Jun 14, 2016

lambday commented Jun 14, 2016

lambday Jun 14, 2016

karlnapf Jun 15, 2016

vigsterkr commented Jun 15, 2016

		@@ -33,6 +33,8 @@

		#include <shogun/mathematics/linalg/SGLinalg.h>

		#include<iostream>

Linalg - Vector Class #3277

Linalg - Vector Class #3277

Conversation

OXPHOS commented Jun 9, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OXPHOS commented Jun 13, 2016 • edited

OXPHOS Jun 13, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lambday commented Jun 14, 2016

lambday commented Jun 14, 2016

OXPHOS commented Jun 14, 2016

lambday commented Jun 14, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vigsterkr commented Jun 15, 2016

OXPHOS commented Jun 9, 2016 •

edited

OXPHOS commented Jun 13, 2016 •

edited

OXPHOS Jun 13, 2016 •

edited