-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[Frontend] Add NNEF frontend #17053
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
base: main
Are you sure you want to change the base?
[Frontend] Add NNEF frontend #17053
Conversation
|
|
||
|
|
||
| def verify_model( | ||
| model_path, |
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
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.
Done in 944db77
…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, |
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.