Skip to content

TensorFlow image_processing component#7083

Merged
arsaboo merged 15 commits intohome-assistant:nextfrom
hunterjm:feature/tensorflow
Nov 5, 2018
Merged

TensorFlow image_processing component#7083
arsaboo merged 15 commits intohome-assistant:nextfrom
hunterjm:feature/tensorflow

Conversation

@hunterjm
Copy link
Copy Markdown
Member

Description:

Adding a TensorFlow Object Detection image_processing component to run either pre-trained or custom built machine learning object detection models through TensorFlow and expose the results to Home Assistant.

Pull request in home-assistant (if applicable): home-assistant/core#17795

Checklist:

  • Branch: next is for changes and new documentation that will go public with the next home-assistant release. Fixes, changes and adjustments for the current release should be created against current.
  • The documentation follows the standards.

@frenck frenck added Hacktoberfest An PR on this issue (or the PR itself) is eligible towards Hacktoberfest! new-integration This PR adds documentation for a new Home Assistant integration ready-for-review This PR needs to be reviewed next This PR goes into the next branch has-parent This PR has a parent PR in another repo and removed to-do labels Oct 25, 2018
frenck
frenck previously requested changes Oct 25, 2018
Copy link
Copy Markdown
Member

@frenck frenck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hunterjm I've reviewed your PR and left some initial comments.

I'm also kinda against the installation instructions for TensorFlow.
e.g.:

  • Some people use venv?
  • Some people Hass.io?
  • How about none 64 bits installations? ARM64?
  • What if versions need upgrading? (e.g., on protobuf)? Who maintains the docs?

I think in general, we should avoid being an instruction manual for third-party software.

If you could take a look? 👍

Comment thread source/_components/image_processing.tensorflow.markdown
Comment thread source/_components/image_processing.tensorflow.markdown Outdated
Comment thread source/_components/image_processing.tensorflow.markdown Outdated
Comment thread source/_components/image_processing.tensorflow.markdown Outdated
Comment thread source/_components/image_processing.tensorflow.markdown Outdated
Comment thread source/_components/image_processing.tensorflow.markdown Outdated
Comment thread source/_components/image_processing.tensorflow.markdown Outdated
Comment thread source/_components/image_processing.tensorflow.markdown Outdated
Comment thread source/_components/image_processing.tensorflow.markdown Outdated
Comment thread source/_components/image_processing.tensorflow.markdown Outdated
@frenck frenck added in-progress This PR/Issue is currently being worked on and removed ready-for-review This PR needs to be reviewed labels Oct 25, 2018
@hunterjm
Copy link
Copy Markdown
Member Author

@frenck - Thank you for the feedback. The detailed instructions are for those using hassbian or venv. @arsaboo is running a venv and they were initially put together to help him set it up. If approved, I would eventually plan to have it pre-configured in the Hass.io image as well.

Currently I am running Hass.io on a NUC8 and have just replaced the home-assistant image with one that was built from my modifications to the Dockerfile. Google does not release a TensorFlow wheel for alpine linux, so that installation becomes more complicated, but I believe it can be tackled as a separate PR. They do have ARM images compatible with the Raspberry Pi.

The install script is specific to x86_64. Maybe instead of providing a script I can outline the dependencies? I felt that leaving it out entirely would create too much confusion, and it really isn't that difficult to setup, but you would need to sort through multiple documents in the tensorflow/models repo to figure it out yourself.

As far as upgrades/versioning, protobuf is just used to build the python models from the protos since TensorFlow is a multi-language library. We could pin the commit to download, the Detection Model Zoo version, and the TensorFlow wheel version in the documentation to have a working version as long as those resources are still available, even if some get updated.

Thoughts?

@arsaboo
Copy link
Copy Markdown
Contributor

arsaboo commented Oct 25, 2018

I think for this PR, we can include brief instructions. We only need instructions for Tensorflow installation (based on the system) and then list the files/dependencies that are required. The detailed instructions (e.g., mv commands) can be provided in a gist, like we do for OpenCV.

@hunterjm
Copy link
Copy Markdown
Member Author

hunterjm commented Oct 25, 2018

What about simply linking to this? Then providing basic instructions on what to do after?

The problem is only half of that is relevant for this use case.

@robmarkcole
Copy link
Copy Markdown
Contributor

robmarkcole commented Oct 26, 2018

I agree that HA specific instructions are preferable to linking to an external site with extraneous info which will cause confusion. However these instructions could be hosted on a gist as @arsaboo suggested. For opencv I usually pip install opencv-python, wouldn't hurt to add?

@balloob balloob added this to the 0.82 milestone Nov 3, 2018
Comment thread source/_components/image_processing.tensorflow.markdown Outdated
Comment thread source/_components/image_processing.tensorflow.markdown Outdated
@hunterjm
Copy link
Copy Markdown
Member Author

hunterjm commented Nov 4, 2018

Comments addressed. I also added a warning to let Hass.io users know that this component will not work out of the box with their setup, and linked to the system requirements of TensorFlow.

Comment thread source/_components/image_processing.tensorflow.markdown Outdated
@hunterjm
Copy link
Copy Markdown
Member Author

hunterjm commented Nov 4, 2018

Finished testing on Hassbian - updated warning to reflect additional package requirements.

Comment thread source/_components/image_processing.tensorflow.markdown Outdated
@robmarkcole
Copy link
Copy Markdown
Contributor

@hunterjm I added a couple of minor comments. I wouldn't worry too much about getting these docs perfect, as we will get a lot of useful feedback when the component is released to the community.

One final comment, we are here using score rather than confidence (a configurable for the face recognition components, also the metric reported by Facebox). I think we should compile a list of these terms and their meanings in the platform page. I see score as being the output from the model, whilst confidence being a threshold we define as being meaningful. @arsaboo what do you think of this suggestion?

@robmarkcole
Copy link
Copy Markdown
Contributor

LGTM @frenck @arsaboo

@arsaboo
Copy link
Copy Markdown
Contributor

arsaboo commented Nov 4, 2018

Looks good to me.

arsaboo
arsaboo previously approved these changes Nov 4, 2018
Comment thread source/_components/image_processing.tensorflow.markdown Outdated
@arsaboo
Copy link
Copy Markdown
Contributor

arsaboo commented Nov 4, 2018

Looks good to me.

@frenck comments are addressed as well (as most of those are install related). I will merge this tomorrow, unless there are some residual concerns.

@arsaboo arsaboo dismissed frenck’s stale review November 5, 2018 14:32

comments addressed.

@arsaboo arsaboo merged commit 897935a into home-assistant:next Nov 5, 2018
@ghost ghost removed the in-progress This PR/Issue is currently being worked on label Nov 5, 2018
@balloob balloob added the cherry-picked This PR has been manually picked and merged into the current branch label Nov 9, 2018
balloob pushed a commit that referenced this pull request Nov 9, 2018
* initial documentation for image_processing.tensorflow

* add raw tag to fix template errors

* changes per review

* update documentation to reflect latest component changes and pull script out to gist

* do not use deps folder as default, as it should only be managed by HA.  Update to have tensorflow in root config directory

* make gist a link, remove additional deps references

* add additional cameras to example config, shorten model selection section, add warning at top for Hass.io

* Update warning adding further Hassbian instructions

* changes per PR review

* fix init location in docs

* fix spelling of raspberry

* Update image_processing.tensorflow.markdown

* Minor changes

* Add class

* add link to tensorflow install site for additional installation options
@hunterjm hunterjm deleted the feature/tensorflow branch November 10, 2018 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cherry-picked This PR has been manually picked and merged into the current branch Hacktoberfest An PR on this issue (or the PR itself) is eligible towards Hacktoberfest! has-parent This PR has a parent PR in another repo new-integration This PR adds documentation for a new Home Assistant integration next This PR goes into the next branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants