-
Notifications
You must be signed in to change notification settings - Fork 158
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
Amend memory hierarchy to enable for push constants and functional interface for more flexible operations #164
Conversation
e7f03a4
to
198fb46
Compare
The new interface looks neat! Just a question
Is it required to be manually created, or optional and can be created automatically? |
@aliPMPAINT great question, the short answer is no, but you can see how it's still possible to delegate the logic to handle SPIRV code as part of the Operations, the example provided is with the OpMult class which rebuilds the algorithm internally, mainly ensuring it's possible to contain the logic to retrieve / load spirv from the class itself: mgr.sequence()
->eval<kp::OpTensorSyncDevice>(params)
->eval<kp::OpMult>(params, mgr.algorithm())
->eval<kp::OpTensorSyncLocal>(params); You can look at the OpMult class, which basically provides its own imlpementation that rebuilds the kp::Algorithm https://github.com/EthicalML/vulkan-kompute/blob/198fb46eb628051dd3a800b586bfca63ceb71aa2/src/include/kompute/operations/OpMult.hpp The only thing required is basically passing an algorithm that contains the respective tensors to use, that's basically it. There were tradeoffs that had to be made, primarily taking into consideration the limitatsion of the vulkan resources (pipeline being tied to descriptorsets) as well as operations not having access to the sequnece / manager. You can see the different interfaces explored but that seemed to be the one that covered all corners. If you are curious on the internals of the new memory hierarchy, this diagram visualises the relationships between the different components as well as the owning relationships between each of the classes: |
Fixes #54
Fixes #12
Fixes: #166
Fixes: #161 (partially)
Fixes: #160
This PR contains a relatively large refactor that amends the memory hierarchies between components, adding the following:
This is what the new interface looks like:
@aliPMPAINT @alexander-g @SimLeek would be great to hear your thoughts on the new interface.
There was quite a lot of different interfaces that "could have been", but I feel like all were explored, many which led to dead ends - people may wonder, "well what about if X was created via Y instead", but it seems like this flow / interface / memory management hierarchy is the one that seems optimal at this point in time. If people are interested of all the brainstorming involved as wel as other interfaces that were explored you can look at this gist (it's not tidy, it's just a dump of all the different interfaces that were attempted) https://gist.github.com/axsaucedo/87bacf8d7d31fa0319e67cf6ff24a026
Outstanding:
Further things to explore: