craftr-build-4.x icon indicating copy to clipboard operation
craftr-build-4.x copied to clipboard

Pools support

Open mcquay239 opened this issue 7 years ago • 2 comments

Hi Niklas!

I also need to customize pools for my pipeline :)

I monkey-patched your Graph class, put an add_pool(self, pool_name, pool_depth) method and exported it. In Craftrfile I explicitely called this method.

https://github.com/mcquay239/craftr/commit/20544ab6f841abcdc665d93215c065a4c6a98e79

But I suppose you planned to do it in a different way. What do I need to implement to create a pull request?

mcquay239 avatar Apr 04 '17 14:04 mcquay239

Actually, what you got there looks good. 😄 You're only missing a genpool(), or so, function in there. I'm thinking of whether it would make sense to have pool names also be automatically prefixed with the name of your Craftr module, or allow multiple modules create pools with the same name, effectively making them the same pool (even if the module's may not know about the other module that uses the same pool).

What do you think? What do you use pools for?

NiklasRosenstein avatar Apr 05 '17 19:04 NiklasRosenstein

Well I'm using pools for a heavy-memory-usage rule in my custom geometric pipeline.

I'm not sure that it's possible to reuse pools transparently. I think that a root pipline author may create some kind of pool objects and pass them to the modules, but I'm not sure. But I think that using the module prefix should be an option (like gtn function calling).

mcquay239 avatar Apr 05 '17 22:04 mcquay239