-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Update tflite wheel version to 1.13.1 #3435
Conversation
@FrozenGene could you review it? |
tutorials/frontend/from_tflite.py
Outdated
pip install tflite-1.13.1-py2-none-any.whl --user | ||
|
||
|
||
or you could clone FrozenGene/tflite repo which contains generated tflite python module |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is better to let users do it themselves. In my opinion, we should restrict the repo under the umbrella of dmlc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
line 57 still shows the option where people can generate tflite themself.
Options are:
- install
tflite
using pip - simply clone
tflite
repo - generate
tflite
usingflatc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. I saw it. My previous meaning is we restrict the repo url all are under the dmlc
, not non-official ( for example FrozenGene/tflite.). However, I couldn't make sure whether we should have this restriction. Need more input from @tqchen @srkreddy1238 @yzhliu @kevinthesun One solution maybe is we upload these generated python files to dmlc/web-data. Or just let users do it themselves like line 57.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, I removed cloning tflite
repo option from the tutorial.
Given that we are not maintaining TFLite. I recommend we keep all the docker builds as binary, give instructions to install tensorflow and host the wheel image somewhere else. |
@tqchen I can replace |
Sorry that I was not being clear. I think we should build from source in the docker script when possible. As the binary wheel distribution is not really official. |
@tqchen currently we use This PR updates tflite wheel to version 1.13.1 - In the tutorial we offer two option on how users can install
In the docker file we use Option2 - If the options listed above are fine with you then can you merge PR dmlc/web-data#190 |
@FrozenGene What you think if you make new release in you repo Assets URLs will be smth like: Looks like PR dmlc/web-data#190 will never be merged. |
Given that we do not maintain tflite, I personally think it is not a good idea to distribute tflite binaries as part of the project. We could certainly advertise |
@FrozenGene I created tflite Release 1.13.1 in my forked repo Can you do the same in your repo? |
Changed |
@apivovarov I think you could make a PR on my tflite repo, then I merge it. If there is a need, I could create a release branch after merging your assets wheel files Or your prefer way is I create one release branch (like 1.13) and upload wheel files as you list? |
@apivovarov I have done it. You could do a double check whether it is you want. |
Thank you Zhao! The release and assets look good! |
Thanks @FrozenGene @apivovarov , this PR is now merged |
This PR updates
tflite
wheel version to 1.13.1I also added alternative way to get
tflite
module by cloning https://github.com/FrozenGene/tflite.git repo.Merge it after web-data PR is merged dmlc/web-data#190