search

toradex / torizon-samples

All sample code related to TorizonCore project.
The Unlicense
32 stars 24 forks source link

tflite: python: tflitePythonVivante: Fix build process and remove pre-built libraries #18

Closed brunoaamello closed 6 months ago

brunoaamello commented 7 months ago

Hello maintainers,

I am creating this pull request to hopefully solve the build issues present in the tflitePythonVivante sample.

This PR also:

This is initially set as a draft because I want to do some more testing to make sure everything works as expected.

leograba commented 7 months ago

I see a change in style, for example on Dockerfiles instead of keeping one package per line on RUN apt install statements, moving to several packages per line.

Not nitpicking on them as there are no agreed rules on style. Just pointing it our so it does not go unnoticed.

leograba commented 7 months ago

@lucasbernardestoradex since you have recently worked on this topic, I'd appreciate you review the changes as well and let us know your thoughts.

charles2910 commented 7 months ago

I see a change in style, for example on Dockerfiles instead of keeping one package per line on RUN apt install statements, moving to several packages per line.

Not nitpicking on them as there are no agreed rules on style. Just pointing it our so it does not go unnoticed.

I would prefer to keep them listed one per line for 2 main reasons (and tabs set to 4 spaces because it seems to what the IDE uses), it's easier to parse the package list that way and it's also easier to see diffs. If you add or remove a package, you get exactly that in the diff. About the other changes in style, I don't have a preferred style, so it's good as is.

andreriesco commented 7 months ago

Hello @brunoaamello, since this template has been added a while ago, can you also update the IDE part of the template?

brunoaamello commented 7 months ago

Thanks for the comments @leograba, @charles2910 and @andreriesco.

I will apply the suggested changes and update the IDE template as requested.

brunoaamello commented 7 months ago

Hello maintainers,

Thanks for your comments and sorry for the delay.

I have updated the Pull Request according to them and tested both the debug and release containers. Everything works as expected with NPU acceleration.

I will remove the "Draft" from the title as I believe it is ready to be reviewed and merged.

andreriesco commented 6 months ago

LGTM