Tensorrt development support - Githubissues

nodefluxio / vortex

A Deep Learning Model Development Framework for Computer Vision

27 stars 6 forks source link

Tensorrt development support #94

Closed alifahrri closed 3 years ago

alifahrri commented 3 years ago

Type of changes

Please check the type of change your PR introduces: - [x] Bugfix - [] Feature - [ ] Code style update (formatting, renaming) - [ ] Refactoring (no functional changes, no api changes) - [x] Build related changes - [ ] Documentation content changes - [ ] Other (please describe): ## What is the current behavior?

Tensorrt dockerfile doesnt support development env. Misleading prediction time profile for pytorch model validation.

fix #93, close #92

What is the new behavior?

Tensorrt dockerfile support development env.
prediction time profile for pytorch model validation includes synchronization

Checklist

[x] I have added necessary documentation (if appropriate)
[x] Any dependent changes have been merged and published in downstream modules

alifahrri commented 3 years ago

could you please add docker build ci?

ok brb fix

triwahyuu commented 3 years ago

is it possible for the development tag to have all runtime including the tensorrt runtime? then the development build from the main dockerfile could be removed. I think that would be easier to use and not having too many tags each release

alifahrri commented 3 years ago

is it possible for the development tag to have all runtime including the tensorrt runtime? then the development build from the main dockerfile could be removed. I think that would be easier to use and not having too many tags each release

the development targget from onnxruntime-tensorrt actually has all runtime (onnxruntime tensorrt, cuda, cpu, and torchscript) available, but additional feature (hyperparam opt) not included

triwahyuu commented 3 years ago

as I understand from the docker build tensorrt for the development, it will overwrite the main vortex development build since it only use the version as tag, and also for latest (e.g. nodefluxio/vortex:0.2.1). won't it be better to just fully support it? since it will be actually be the main images.

alifahrri commented 3 years ago

may be i misunderstood tag in the workflow file, turns out it is just vortex version like you said. But anyway I think it is better to support separately at least for now since this kind of workflow took to long to build

triwahyuu commented 3 years ago

oh I see, okayy. then you will change the tag name for the tensorrt development right?

triwahyuu commented 3 years ago

it seems like you're not using paralel compilation when compiling the onnxruntime, that's why it takes way too long to complete. I think you could use the --parallel argument for it. reference from this https://github.com/microsoft/onnxruntime/blob/master/build.sh

alifahrri commented 3 years ago

it seems like you're not using paralel compilation when compiling the onnxruntime, that's why it takes way too long to complete. I think you could use the --parallel argument for it. reference from this https://github.com/microsoft/onnxruntime/blob/master/build.sh

Ok, but since the builder only has two cores, this will halved the build time at best

triwahyuu commented 3 years ago

well at least it way better than before. so could you support for the hypopt as well for the development?

triwahyuu commented 3 years ago

I think that's all for the the review, you can merge this