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

Make device properties prettier #210

Open
leofang opened this issue Nov 1, 2024 · 5 comments · May be fixed by #409
Open

Make device properties prettier #210

leofang opened this issue Nov 1, 2024 · 5 comments · May be fixed by #409
Assignees
Labels
cuda.core Everything related to the cuda.core module enhancement Any code-related improvements P1 Medium priority - Should do

Comments

@leofang
Copy link
Member

leofang commented Nov 1, 2024

Similar to #205 but for Device properties. Currently we return all of them at once, with the C enumerator names as the keys

@property
def properties(self) -> dict:
# TODO: pythonize the key names
return handle_return(cudart.cudaGetDeviceProperties(self._id))

We should perhaps make the returned object a Python class instead of dict, with nice, pythonic attributes.

@leofang leofang added cuda.core Everything related to the cuda.core module enhancement Any code-related improvements P0 High priority - Must do! labels Nov 1, 2024
@leofang leofang added this to the cuda.core beta 2 milestone Nov 1, 2024
@leofang
Copy link
Member Author

leofang commented Nov 1, 2024

xref: our C++ counterpart (cudax) NVIDIA/cccl#2084

@leofang leofang added P1 Medium priority - Should do and removed P0 High priority - Must do! labels Nov 14, 2024
@ksimpson-work ksimpson-work linked a pull request Jan 17, 2025 that will close this issue
@ksimpson-work
Copy link
Contributor

Piotr suggested we base the exposure on cudaDeviceAttribtues rather than cudaDeviceProperties to reduce the minor version compatibility considerations. WDYT @leofang

@leofang
Copy link
Member Author

leofang commented Jan 18, 2025

Inside you DeviceProperties class we could consider using that. But I don't know how does this help with

to reduce the minor version compatibility considerations

@ksimpson-work
Copy link
Contributor

I guess if cuda 13.x.y came out and .y had some great new property we wanted to expose, we would have to wait for 13.z if we used the property struct in the backend, whereas we could expose it right away if we used the attributes. I will look into it and wee what the trade-offs are

@leofang
Copy link
Member Author

leofang commented Jan 20, 2025

Yeah followed-up internally, the device prop struct is frozen by major.0, so any new attributes added in major.minor will not be exposed to the prop struct until the next major release, whereas the attribute query API can immediately gain support by the minor release that adds the new attribute. Let's move away from the device prop struct, and query & cache the attributes lazily (on the as-needed basis).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cuda.core Everything related to the cuda.core module enhancement Any code-related improvements P1 Medium priority - Should do
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants