search

craftr-build / craftr-build-4.x

Frontend for the Craftr build framework.
https://craftr-build.github.io/craftr/
Other
60 stars 14 forks source link

Pools support #175

Open mcquay239 opened 7 years ago

mcquay239 commented 7 years ago

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?

NiklasRosenstein commented 7 years ago

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?

mcquay239 commented 7 years ago

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).