Allow any GPU resource types support

[typhoonzero]

What changes were proposed in this pull request?

The K8s cluster used for GPU training may have other GPU resource types other than nvidia.com/gpu, there are other vendors like AMD or opensource vGPU implementations, like https://github.com/4paradigm/k8s-vgpu-scheduler, https://github.com/tkestack/gpu-manager etc. I propose to add support for setup user inputed resource types.

How was this pull request tested?

Tests added at test_pipeline_constructor.py

TODO:
Docs should updated in another PR.

[elyra-bot]

Thanks for making a pull request to Elyra!

To try out this branch on binder, follow this link:

[kevin-bates]

Although gpu_vendor was not a member variable previously, we should set the member. Also, for consistency, we should reference gpu_limit via self.

Suggested change

	gpu_vendor = self.pipeline_envs.get("GPU_VENDOR", "nvidia")
	self.container.set_gpu_limit(gpu=str(gpu_limit), vendor=gpu_vendor)
	self.gpu_vendor = self.pipeline_envs.get("GPU_VENDOR", "nvidia")
	self.container.set_gpu_limit(gpu=str(self.gpu_limit), vendor=self.gpu_vendor)

[kevin-bates]

Typo

Suggested change

	if not self.gpu_memory_vendor:
	raise ValueError("gpu_memory_vendor not seted while gpu_memory is specified > 0")
	if not self.gpu_memory_vendor:
	raise ValueError("gpu_memory_vendor not set while gpu_memory is specified")

Question:
This PR is making gpu_memory_vendor a required value. Is there a reasonably common default (like "nvidia" is for gpu_vendor) that we can fall back to? Are the same vendor names used for gpu_limit and gpu_memory? I.e., can we use env GPU_VENDOR here as well if not provided by the user?

[akchinSTC]

"nvidia" I would consider to be the default due to the market share they hold. Not sure about the vendor spec when it comes to gpu limit and memory, I always assumed these would be agnostic?

[typhoonzero]

Question:
This PR is making gpu_memory_vendor a required value. Is there a reasonably common default (like "nvidia" is for gpu_vendor) that we can fall back to? Are the same vendor names used for gpu_limit and gpu_memory? I.e., can we use env GPU_VENDOR here as well if not provided by the user?

Thanks for reviewing!

Actually, gpu_vendor, gpu_memory and gpu_memory_vendor are ment to be optional. I'm looking for a way to make "nvidia" as default, as it is before, and make gpu_vendor, gpu_memory and gpu_memory_vendor to be optional.

[typhoonzero]

By the way, I've tested we can leave gpu_vendor, gpu_memory and gpu_memory_vendor blank, set gpu to 1, we'll have a step with default gpu resource setup like nvidia.com/gpu: 1.

[ptitzler]

Please make sure that Kubeflow Pipelines actually accepts custom values. Elyra uses ContainerOp to implement generic components (running notebooks etc), which seems to limit vendors to nvidia and amd (search for def set_gpu_limit in above link).

[typhoonzero]

Please make sure that Kubeflow Pipelines actually accepts custom values. Elyra uses ContainerOp to implement generic components (running notebooks etc), which seems to limit vendors to nvidia and amd (search for def set_gpu_limit in above link).

Thanks for reminding, I'm using ContainerOp's member add_resource_limit to add any resource types, see:

https://github.com/elyra-ai/elyra/pull/2973/files#diff-8baadb91039b47b451a9ecb6a166cbcddbe75bb825776ab4eac9d7c9e6f9170cR274-R275

[akchinSTC]

Hi @typhoonzero, thanks for the contribution.
Things looking good, looking for more opinions from other members. For me, im wondering if there is a way to put these new parameters under a "click to show" type of button since these fields are starting to take up a lot of real estate.

[akchinSTC]

is the gpu_memory_vendor param exclusive to virtualized GPU resources? If so, can we change the name to something like vgpu.....

[typhoonzero]

Moved to #3029