[Frontend] Add NNEF frontend#17053
Conversation
|
|
||
|
|
||
| def verify_model( | ||
| model_path, |
There was a problem hiding this comment.
Historically we test correctness via the execution results. This however, leads to compound running time of the tests, and less unit testcases. As such tests needs to run for frontend/backend pairs.
As the intermediate operators getting stablizes, we are getting to a world where we can unit test the frontend conversion independently. By validating they goes into the right structural form of IR. The backend correctness are then tested separately.
So we would now instead move toward structural equality tests for unit tests when bringing in new frontends, to reduce the cost in that front. See the FX importer for an example.
https://github.com/apache/tvm/blob/main/tests/python/relax/test_frontend_from_fx.py#L39
The execution tests can still be used, please bring them to python/nightly/frontend, so they won't be triggered per PR
There was a problem hiding this comment.
Should I also add structural tests for Relay, or would you prefer Relax only? In that case execution tests would exist for both frontends under tvm-root/tests/python/nightly/frontend/, and the structural Relax tests would be kept in the normal folder, tvm-root/tests/python/relax/.
If structural Relay tests are preferable, what IRModule creating method is recommended for Relay creation? Building a relay.Function, and comparing that with the frontend's module? Can I follow this example, and compare a span filled, with a non-span filled graph? https://github.com/apache/tvm/blob/main/tests/python/frontend/pytorch/test_fx_quant.py#L42
Also, how should I create the directory structure under nightly/frontend? Should I separate Relay from Relax under a similar structure to regular tests? Or do you have a system in mind?
There was a problem hiding this comment.
if we want to keep execution tests in relay, please put them under nightly/frontend. Since relay creation can be harder
For relax, we can do structure test
…ural, Move Relay tests to nightly
|
@agoston-mc It would be good to submit the Docker-related changes (the changes under directory docker/ in this PR) as a separate PR and get it merged first. Then, we can use the newly built image in this PR to pass the CI checks. You can find some image doc here: https://tvm.apache.org/docs/contribute/ci.html?highlight=update%20container#docker-images. |
# Conflicts: # ffi/scripts/run_tests.sh # python/tvm/meta_schedule/post_optimization/__init__.py # python/tvm/meta_schedule/post_optimization/nnef.py # python/tvm/meta_schedule/post_optimization/nnef_ops.py # tests/scripts/task_python_frontend.sh # tests/scripts/task_python_nightly.sh
|
@tvm-bot rerun |
|
Hi, |
|
Hello, |
|
@tvm-bot rerun |
|
Thanks a lot for the ping and sorry about the delayed note. We have been reflecting about the overall strategy of tvm support onwards enabling lean core and enable more thirdparty build on top so that we have confidence in the cores we build and reduce overall maintainace surface. After reviewing the state of NNEF and its current adoption, i feel it benefit from staying offtree for now, in places like https://github.com/KhronosGroup/NNEF-Tools instead. |
3 participants
Add an NNEF frontend to Relax and Relay, as proposed in RFC #108.
The Docker image scripts and test scripts have also been extended to accommodate NNEF.