Device Properties #184

alexander-g · 2021-03-12T07:38:49Z

Added Manager::getDeviceProperties() which exposes some device parameters that are relevant for Kompute

devprops = mgr.get_device_properties()
print(devprops)

Device Name:                   SwiftShader Device (LLVM 10.0.0)
Maximum Workgroup Count:       65535, 65535, 65535
Maximum Workgroup Invocations: 128
Maximum Workgroup Size:        128, 128, 64
Timestamps Supported:          False

(Single Include header changed more than I want, not sure why.)

axsaucedo

I have actually been thinking of convert a lot of the Manager functions related to vulkan resource creation into static functions, mainly as the creation of the Device, etc could be quite useful for the unit tests to test Tensors / other components in isolation without having to create a manager, so I think this is an interesting approach.

I added a couple of comments, the main one is to avoid creating structs and instead returning the vk:: objects instead in the C++, and returning a python Dict in the bindings instead of creating a new class binding

axsaucedo · 2021-03-12T07:56:08Z

src/Manager.cpp

+DeviceProperties Manager::getDeviceProperties() const
+{
+  const vk::PhysicalDeviceProperties properties = this->mPhysicalDevice->getProperties();
+  const DeviceProperties output{


Why a new struct when we can just return a vk::PhysicalDeviceProperties? The PhysicalDeviceLimits object itself has a very long list of limits so it's a slippery slope exposing only a subset of these, as later on more will be required to the extent it's better to just expose the object itself

Is it because creating a PyBind binding to the object is harder? Perhaps the subset of properties exposed can just be on the Python binding as opposed to the underlying component

The idea was that most of the properties are not relevant for Kompute. Moreover, some of the properties could be post-processed for easier access by the user, e.g. amount of device memory: Using the raw vk:: struct one would need to find the correct memory type first.

Not sure I understand, what would be the requirements to find the correct memory type? I can see that you read for example properties.limits.maxComputeWorkGroupCount, is the memory type whether it's not of type Compute? I agree that there is a large number of options on the device properties, I'm just thinking that having a struct in between may result in more abstractions to maintain and potentially people requesting more to be exposed - what about the suggestion on having it in the python binding as a python dict? It could be buitl as a single-level python dict, where the key could just be limits_max_compute_work_group_count, etc

This is not yet implemented. I mean that the the user would need to iterate over the memory types as in Tensor::allocateBindMemory to find the correct memory heap that is used for the buffers.

axsaucedo · 2021-03-12T07:58:16Z

single_include/kompute/Kompute.hpp

-    uint32_t size() {
-        return this->mSize;
-    }
+    uint32_t size() { return this->mSize; }


Did you run make format? or why is the kompute.hpp modifying the format?

What's the version of your CLIs / dependencies related to the creation of the single header?

make format uses hardcoded paths. I've manually run clang-format as in the Makefile, didn't help.

Versions:
clang-format 10.0
quom 1.2.0

axsaucedo · 2021-03-12T08:01:35Z

python/src/main.cpp

@@ -217,7 +217,26 @@ PYBIND11_MODULE(kp, m) {
            py::arg("spirv"),
            py::arg("workgroup") = kp::Workgroup(),
            py::arg("spec_consts") = kp::Constants(),
-            py::arg("push_consts") = kp::Constants());
+            py::arg("push_consts") = kp::Constants())
+        .def("get_device_properties", &kp::Manager::getDeviceProperties, "Return a struct containing information about the device");


Is there a way to just return the device properties as a Dict instead of a new class? Conscious if we start exposing Vk classes it could lead into more vk classes being requested

axsaucedo

Interesting - thanks for the pr.

I have actually been thinking of converting a lot of the Manager functions related to vulkan resource creation into static functions, mainly as the creation of the Device, etc could be quite useful for the unit tests to test Tensors / other components in isolation without having to create a manager, so I think this is an interesting approach.

I added a couple of comments, the main one is to avoid creating structs and instead returning the vk:: objects instead in the C++, and returning a python Dict in the bindings instead of creating a new class binding

alexander-g · 2021-03-12T08:53:13Z

Modified to return raw vk:: struct and Python dict

mgr.get_device_properties()

{'device_name': 'SwiftShader Device (LLVM 10.0.0)',
 'max_work_group_count': (65535, 65535, 65535),
 'max_work_group_invocations': 128,
 'max_work_group_size': (128, 128, 64),
 'timestamps_supported': False}

axsaucedo

Thanks @alexander-g, python dictionaries look great in C++ with the pybind literals

axsaucedo · 2021-03-12T08:56:15Z

python/src/main.cpp

+            const auto properties = self.getDeviceProperties();
+            py::dict py_props(
+                "device_name"_a = std::string(properties.deviceName.data()),
+                "max_work_group_count"_a       = py::make_tuple(properties.limits.maxComputeWorkGroupCount[0],


Device Properties

f52efce

axsaucedo requested changes Mar 12, 2021

View reviewed changes

raw vk::deviceproperties and python dict

d71d169

axsaucedo approved these changes Mar 12, 2021

View reviewed changes

axsaucedo merged commit abd635c into KomputeProject:master Mar 12, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Device Properties #184

Device Properties #184

alexander-g commented Mar 12, 2021

axsaucedo left a comment

axsaucedo Mar 12, 2021

axsaucedo Mar 12, 2021

axsaucedo Mar 12, 2021

alexander-g Mar 12, 2021

axsaucedo Mar 12, 2021 •

edited

Loading

alexander-g Mar 12, 2021

axsaucedo Mar 12, 2021

axsaucedo Mar 12, 2021 •

edited

Loading

alexander-g Mar 12, 2021

axsaucedo Mar 12, 2021

axsaucedo left a comment

alexander-g commented Mar 12, 2021

axsaucedo left a comment

axsaucedo Mar 12, 2021

Device Properties #184

Device Properties #184

Conversation

alexander-g commented Mar 12, 2021

axsaucedo left a 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

axsaucedo Mar 12, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

axsaucedo Mar 12, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

axsaucedo left a comment

Choose a reason for hiding this comment

alexander-g commented Mar 12, 2021

axsaucedo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

axsaucedo Mar 12, 2021 •

edited

Loading

axsaucedo Mar 12, 2021 •

edited

Loading