KomputeProject / kompute

General purpose GPU compute framework built on Vulkan to support 1000s of cross vendor graphics cards (AMD, Qualcomm, NVIDIA & friends). Blazing fast, mobile-enabled, asynchronous and optimized for advanced GPU data processing usecases. Backed by the Linux Foundation.
http://kompute.cc/
Apache License 2.0
1.97k stars 151 forks source link

Issue #925: Added noncopy constructor for Tensor and TensorT #369

Closed ThePseudo closed 1 month ago

ThePseudo commented 5 months ago

Added methods reserve() and constructor for Tensor and TensorT

axsaucedo commented 4 months ago

Looks good - only thing would be to add a test, and we can then merge. You can add this on the Tensor test file.

axsaucedo commented 2 months ago

Just following up, do you think you would be able to have a look with a short test @ThePseudo ?

ThePseudo commented 2 months ago

Sorry for the delay, at some point stuff happened and I lost track of a few things (one being this). I created a short test, which could be expanded upon, but is basically "inspired" by the other test already present. I hope this works!

axsaucedo commented 2 months ago

No worries @ThePseudo, thank you for following up - running test suite

axsaucedo commented 2 months ago

@ThePseudo it seems tests failing, did you run locally?

ThePseudo commented 2 months ago

The terrible thing is that I am not really an expert in the Google test suite... but I found that the tests were failing for a good reason, apparently, and it is building. Which is strange, since "it worked on my machine", but let's try this out

ThePseudo commented 1 month ago

Thank you @ThePseudo - remaining commment on a small docstring change required

It should be done now, but tell me if the format is not correct (I took inspiration from previous docstrings)

axsaucedo commented 1 month ago

Added extra suggestion as it needs to follow the same format as rest of docstrings

axsaucedo commented 1 month ago

Thank you @ThePseudo - signed commit required, and we should be able to wrap it up :)

ThePseudo commented 1 month ago

Thank you @ThePseudo - signed commit required, and we should be able to wrap it up :)

Forgot it, now added! :D

axsaucedo commented 1 month ago

Thanks!