Validate checkpoint GPU UUID inputs early#2086
Conversation
Signed-off-by: Aryan <aryansputta@gmail.com>
There was a problem hiding this comment.
Hi @aryanputta as you might have seen in your other PRs, we generally do not accept mock tests like this. Please make sure new/revised tests fit in the existing test style, and GPU tests actually pass locally before submitting.
| if isinstance(value, driver.CUuuid): | ||
| return value |
There was a problem hiding this comment.
Actually, reading this more carefully: What is the motivation for accepting CUuuid here? Do you have a use case?
Note that Device.uuid returns a string:
https://nvidia.github.io/cuda-python/cuda-core/latest/generated/cuda.core.Device.html#cuda.core.Device.uuid
|
@leofang You were right on both points here. I over-generalized the helper instead of matching the actual cuda.core API surface. What I changed in commit 083e411:
On the motivation question specifically: I do not have a strong public-facing use case for accepting CUuuid objects here. The intended caller-facing path is the string UUID returned by Device.uuid, and the existing real checkpoint tests already follow that contract. For example, cuda_core/tests/test_checkpoint.py builds migration mappings from devices[i].uuid in _build_rotation_mapping() and runs the driver-backed checkpoint/restore scenarios without mocks. I also checked local validation from this environment. Syntax validation passed, but I could not run the checkpoint pytest module end-to-end here because test import setup fails locally with ModuleNotFoundError: cuda.pathfinder before reaching the GPU-backed scenarios. So I did not keep or add any mock-style substitute tests for this change. |
Summary
Testing