Subclass MLGraph based on the context that creates it

[huningxin]

Split from #341 (comment), where @wacky6 mentioned

Maybe we should subclass MLGraph based on the context that creates it. For example, CPU context returns a MLCpuGraph with compute(). GPU context returns a MLGpuGraph with compute() and GPU interop methods (commandBuffer, dispatch, etc).

If we fold the command recording methods into MLGpuGraph, it may not support recording multiple MLGraphs into one command buffer that MLCommandEncoder supports. Pipelining models execution may reduce the GPU queue submission overhead and improve the throughput.

/cc @wchao1115

[anssiko]

I'm hearing this redesign would impact the current MLCommandEncoder design. I'm proposing to revisit this issue when we have received further implementation experience and user feedback for the current MLCommandEncoder interface.

Perhaps we are able to address this issue with better example code on how to use the GPU command encoder.

[zolkis]

What about including the graph as internal slot to context as barinstormed in #303.
(Note that whether builder is also part of context, is a different issue, it may not necessarily need to be).

As for command encoder, there opportunities to simplify that, too, for instance some brainstorming in #333.
Also related to #322.

I agree we should address these points together when more impl experience/clarity is available.

[a-sully]

Does subclassing MLGraph still provide value if MLCommandEncoder does not exist?

My reading of this issue is that it has so far provided the following reasons for subclassing MLGraph:

1. Creating an MLCommandEncoder from an MLGraph only makes sense if the MLGraph will execute on GPU (and the GPU)
2. You may call compute() if the MLGraph will execute on CPU, or dispatch() if the MLGraph will execute on GPU

With regards to (1), the MLCommandEncoder proposal is no longer being pursued (#528). This is no longer relevant

With regards to (2), I see a few problems to this. Would we have to add another subclass to support an NPU backend? This does not seem sustainable. IMHO it would be nice to have one method to perform "execute the graph", regardless of the backend, and MLBuffer might provide a path forward for this. But for the sake of keeping this issue on track I think the details are best left for another issue :)

---

Are there other reasons to want this subclassing? If not, I would like to advocate for closing this issue

[inexorabletash]

+1 to closing

[zolkis]

IMHO it would be nice to have one method to perform "execute the graph", regardless of the backend

+1 for that.

[huningxin]

+1 to closing this issue.