getnamo
commented
5 years ago Sorry for a late reply, this issue somehow flew under my radar completely.
Let me talk about my motivations for the fork first so you can figure out what would suit you best. The main changes done in this fork is largely to
1) Allow pip to auto-install dependencies. This allows tensorflow updates to be a simple update of https://github.com/getnamo/tensorflow-ue4/blob/master/Content/Scripts/upymodule.json 2) Encapsulate/embedd everything so it doesn't mess with other python installations in your system. 3) Related to 1) ensure that other plugins can specify their dependencies, scripts, and content without needing to be directly placed in a project or the unreal python plugin content folder. In this case the tensorflow plugin has specific tensorflow python scripts that are embedded under the plugin content/scripts and it all gets copied and packaged correctly and you could feasibly write another dependent plugin without changes to this unrealenginepython fork. 4) Ensure all tensorflow tasks can be called on a background thread and returned with results on the gamethread, allowing minimal impact on gamethread. 5) Provide scripts that handle some hard parts e.g. pip, imports, threads, and additional startup, found here: https://github.com/getnamo/UnrealEnginePython/tree/master/Content/Scripts
The things that break the tensorflow plugin are mainly the 3, 4, and 5 changes. If you can replace all the differing dependencies found in https://github.com/getnamo/tensorflow-ue4/blob/master/Content/Scripts/TensorFlowComponent.py you should be able to setup your own fork/change to work with the rest of the plugin. Specifically it's upythread.py for background threading and the custom c++ based python function ue.run_on_gt which handles callbacks. The embedding (2) is not strictly necessary and some devs have changed things to point to their main python installation without issue and conversely pip (1) installation can be handled manually if desired.
Checking your link https://github.com/20tab/UnrealEnginePython/compare/master...getnamo:master, it appears the two are currently compatible. I generally just periodically sync with the master, but I'm not convinced all my changes make sense for the upstream fork. It would require some splitting to figure out what minimal changes would make sense to merge upstream without poluting it with opinionated changes such as embedding or plugin script isolation. If the upstream comes up with better suited solutions to 1-5, I'll gladly replace the workaround ones used here.
I'm open to suggestions, but I'll likely default to the simpler path for the moment due to time limitations.
Hi Jan,
I was using your https://github.com/getnamo/tensorflow-ue4 inside my project. Which required https://github.com/getnamo/UnrealEnginePython/ instead of https://github.com/20tab/UnrealEnginePython/
Now I already had my own fork, but since the changes were small I just did a hard reset to your version and redid the changes.
After some time I upgraded my Unreal Engine. Of course this did not work with your specific fork. So I disabled the tensorflow plugin and switched back to the original '20tab/UnrealEnginePython/'. The main reason for switching back was that Robert (20tab) pulled my changes and my tensorflow code was not yet fully implemented.
Now I would like to use Tensorflow, but it seems your fork is not compatible with the original master, check: https://github.com/20tab/UnrealEnginePython/compare/master...getnamo:master
Side Note: To avoid future headache (and typing commands) I automated updating my fork so that it is exactly same as 20tab, by using a cron command with: git pull 20tab master --rebase git push (20tab is my remote name, use 'remove -v' to check your remote name)
Now I saw your comment here about creating a clean pull request: https://github.com/20tab/UnrealEnginePython/pull/539#issuecomment-432776153
And so I wondered if you would be willing to cleanup your fork as well ^^. So in that case we can use merge / fast forward again. (and perhaps create a pull on 20tab fork with all your changes, so that all forks stay in sync)
Looking at the compare https://github.com/20tab/UnrealEnginePython/compare/master...getnamo:master
It seems that sometimes you pull / merge within GitHub.com (e.g it shows 'verified'), while other times you did it using a git cmd. Personally I stopped using GitHub.com pull / merge, since it contaminated the history with 'Merge branch 'master' of into '.
Now I'm not a git guru by any means, but I think it should be quite easy to fix.
Regard,
TQwan
Note: Sorry for the long post.