[GitHub] [incubator-tvm] trevor-m commented on pull request #6395: [BYOC][TensorRT] TensorRT BYOC integration

2020-10-19 Thread GitBox
trevor-m commented on pull request #6395: URL: https://github.com/apache/incubator-tvm/pull/6395#issuecomment-712487066 CI has passed with USE_TENSORRT_CODEGEN ON since the new CI container is used. This is an automated

[GitHub] [incubator-tvm] trevor-m commented on pull request #6395: [BYOC][TensorRT] TensorRT BYOC integration

2020-10-13 Thread GitBox
trevor-m commented on pull request #6395: URL: https://github.com/apache/incubator-tvm/pull/6395#issuecomment-707923371 > I think we can enable the test and merge after #6679 is landed since its pretty close already? Agreed, makes sense. I'll renable test once #6679 is merged.

[GitHub] [incubator-tvm] trevor-m commented on pull request #6395: [BYOC][TensorRT] TensorRT BYOC integration

2020-10-13 Thread GitBox
trevor-m commented on pull request #6395: URL: https://github.com/apache/incubator-tvm/pull/6395#issuecomment-707919898 > I agree that we can merge it first. But before that, @trevor-m could you rebase against the master and run the tests again locally to see if all of them pass? I am not

[GitHub] [incubator-tvm] trevor-m commented on pull request #6395: [BYOC][TensorRT] TensorRT BYOC integration

2020-10-12 Thread GitBox
trevor-m commented on pull request #6395: URL: https://github.com/apache/incubator-tvm/pull/6395#issuecomment-707363920 > @trevor-m I don't see anything weird with your build rules, but I wonder if changing the cmake config affected something. do you have >1 CI failure showing the

[GitHub] [incubator-tvm] trevor-m commented on pull request #6395: [BYOC][TensorRT] TensorRT BYOC integration

2020-10-12 Thread GitBox
trevor-m commented on pull request #6395: URL: https://github.com/apache/incubator-tvm/pull/6395#issuecomment-707258309 Hey @areusch I found that the failures for `Test MISRA-C runtime` are related to my new CMake option `USE_TENSORRT_CODEGEN`. When `USE_TENSORRT_CODEGEN` is `ON`, the

[GitHub] [incubator-tvm] trevor-m commented on pull request #6395: [BYOC][TensorRT] TensorRT BYOC integration

2020-09-29 Thread GitBox
trevor-m commented on pull request #6395: URL: https://github.com/apache/incubator-tvm/pull/6395#issuecomment-701059299 > @trevor-m i agree this seems unrelated. can you try retriggering the CI just to double-check it's reproducible? there was an issue with flaky bundle_deploy tests a

[GitHub] [incubator-tvm] trevor-m commented on pull request #6395: [BYOC][TensorRT] TensorRT BYOC integration

2020-09-29 Thread GitBox
trevor-m commented on pull request #6395: URL: https://github.com/apache/incubator-tvm/pull/6395#issuecomment-701036415 It looks like `/tests/scripts/task_cpp_unittest.sh` is failing at "Test MISRA-C runtime": ``` python3 build_model.py -o build --test INFO:compile_engine:Using

[GitHub] [incubator-tvm] trevor-m commented on pull request #6395: [BYOC][TensorRT] TensorRT BYOC integration

2020-09-22 Thread GitBox
trevor-m commented on pull request #6395: URL: https://github.com/apache/incubator-tvm/pull/6395#issuecomment-696860539 > Finished reviewing the C++ part and the tests. I am not familiar with the TRT APIs so I didn't review the op converter in details. > > For tests, how long does

[GitHub] [incubator-tvm] trevor-m commented on pull request #6395: [BYOC][TensorRT] TensorRT BYOC integration

2020-09-22 Thread GitBox
trevor-m commented on pull request #6395: URL: https://github.com/apache/incubator-tvm/pull/6395#issuecomment-696860539 > Finished reviewing the C++ part and the tests. I am not familiar with the TRT APIs so I didn't review the op converter in details. > > For tests, how long does

[GitHub] [incubator-tvm] trevor-m commented on pull request #6395: [BYOC][TensorRT] TensorRT BYOC integration

2020-09-15 Thread GitBox
trevor-m commented on pull request #6395: URL: https://github.com/apache/incubator-tvm/pull/6395#issuecomment-693018923 There appears to be an inconsistency in the CI between `tests/lint/cpplint.sh` and `tests/lint/clang_format.sh` for this macro definition: ``` #define

[GitHub] [incubator-tvm] trevor-m commented on pull request #6395: [BYOC][TensorRT] TensorRT BYOC integration

2020-09-15 Thread GitBox
trevor-m commented on pull request #6395: URL: https://github.com/apache/incubator-tvm/pull/6395#issuecomment-692843402 > Just a couple of minor suggestions and comments - looks good overall, without regard to comments/concerns above. > > One overall suggestion from a users

[GitHub] [incubator-tvm] trevor-m commented on pull request #6395: [BYOC][TensorRT] TensorRT BYOC integration

2020-09-14 Thread GitBox
trevor-m commented on pull request #6395: URL: https://github.com/apache/incubator-tvm/pull/6395#issuecomment-692319247 > > I have already created an API to retrieve the TRT version if TVM is compiled with the TRT runtime enabled. However, one of our use cases is to use TVM on a CPU-only

[GitHub] [incubator-tvm] trevor-m commented on pull request #6395: [BYOC][TensorRT] TensorRT BYOC integration

2020-09-14 Thread GitBox
trevor-m commented on pull request #6395: URL: https://github.com/apache/incubator-tvm/pull/6395#issuecomment-692316634 > > Hmm, this seems like it would make the job of the `PruneTensorRTSubgraph` pass much more difficult. `PartitionGraph` already takes care of collecting the inputs and

[GitHub] [incubator-tvm] trevor-m commented on pull request #6395: [BYOC][TensorRT] TensorRT BYOC integration

2020-09-14 Thread GitBox
trevor-m commented on pull request #6395: URL: https://github.com/apache/incubator-tvm/pull/6395#issuecomment-692298168 > For the rest 2 points. > > 2. Is that possible to move the pass before partitioning but after merge compiler region (like `PruneTesnorRTCompilerRegion`)?

[GitHub] [incubator-tvm] trevor-m commented on pull request #6395: [BYOC][TensorRT] TensorRT BYOC integration

2020-09-14 Thread GitBox
trevor-m commented on pull request #6395: URL: https://github.com/apache/incubator-tvm/pull/6395#issuecomment-692256923 Thanks everyone for the great feedback! I've been busy lately, but I plan to start addressing the comments this week.

[GitHub] [incubator-tvm] trevor-m commented on pull request #6395: [BYOC][TensorRT] TensorRT BYOC integration

2020-09-03 Thread GitBox
trevor-m commented on pull request #6395: URL: https://github.com/apache/incubator-tvm/pull/6395#issuecomment-686770757 > > ``` > > 1. Currently, I'm using environment variables to pass these from python to the codegen in C++. I wonder if there is a better way to do this? > > ```