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

From clsparse<Vector><Scalar>Private to clsparse::vector<T> #48

Closed
jpola opened this issue Jun 1, 2015 · 3 comments
Closed

From clsparse<Vector><Scalar>Private to clsparse::vector<T> #48

jpola opened this issue Jun 1, 2015 · 3 comments

Comments

@jpola
Copy link
Contributor

jpola commented Jun 1, 2015

Hi,
currently I'm working on clsparse::vector class which might help us to simplify the code and hide differences between cl1.2 and cl2.0 implementations. I would like to know your opinion about the idea of substituting *Private structures with the clsparse::vector class. If we will have clsparse::vector I don't see the need for the *Private classes to exists. If we want to keep both structures, for all algorithm I will have to write proper specialisations which actually will differ in very little details like passing buffer to kernel. I would like to avoid this unnecessary duplication. The clsparse::vector class is very simple now but is enough, in my opinion, to substitute *Private structures.

pros:

  • unify the code,
  • unify the way to pass the buffers to kernels,
  • manages memory for internal buffers.
  • clsparse::vector == clsparseVectorPrivate
  • clsparse::vector[1] == clsprseScalarPrivate
  • easy to use
  • hides the cl1.2 and cl2.0 differences.
  • good base line to extend this class for other algorithm requirements (iterators)

cons:

  • don't know yet if that will interfere with the C interface. I think it will not for the moment.
  • not quite an issue but a all algorithms need to be refractored.

What is your opinion?

@kknox
Copy link
Contributor

kknox commented Jun 1, 2015

I agree with this general direction. However, I do not think that it can replace ALL of the clsparse private structs, as they contain a lot of host data. For sure, I would hope that they could be used for all cl_mem or void* members.

You may have to create a special constructor, which can 'give' already allocated device memory to the vector. The destructor would also have to be special, because it does not in fact 'own' the memory. In a way, this is kind of like a weak_ptr concept from shared_ptr model. However, when we create temporary internal buffers, we obviously want it to allocated and deallocate the memory because it 'owns' the memory.

@jpola
Copy link
Contributor Author

jpola commented Sep 1, 2015

The clsparse::vector and array are already present in the code. In every iteration I see the progress where the clsparse::vector is overtaking the *Private concept.

@jpola jpola closed this as completed Sep 1, 2015
@kknox
Copy link
Contributor

kknox commented Sep 2, 2015

Yes, and this is a good thing. If we could eventually eliminate the *Private and the memmapRAII classes, clsparse becomes simpler to maintain. With respect to your question in #138, bolt::device_vector would potentially replace clsparse::vector.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants