-
Notifications
You must be signed in to change notification settings - Fork 89
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
Enable Virtual Environments in testing #1322
Comments
The virtual environments need to be setup as a test command, but needs to run before the python onnx tests. Its probably best to use ctest's fixtures to do this(although we dont need a cleanup) to ensure it will always set up the virtual env first. We can probably add a function to setup a virtualenv fixture like this: function(add_py_venv_fixture FIXTURE_NAME VIRTUAL_ENV_DIR REQUIREMENTS_FILE)
...
endfunction() Then Here is a blog about ctest's fixtures: https://crascit.com/2016/10/18/test-fixtures-with-cmake-ctest/ |
Draft PR is up for questions/feedback: #2191 |
During discussions with Paul, he suggested to disable |
Reopened as changes in the other PR for virtual env, once it is ready, need to be merged in as well. |
Addressed review comments on the draft PR. It is up for review again. In the meanwhile, I am testing things out on sles docker container. |
Discussed the duplication of test names issue and why I was setting up virtual env for different Python versions with Paul on Friday. He suggested to check for existence of tests before setting up virtual env to avoid the unique name issue. I have made the changes and put up a PR for review. |
Regarding the requirements file for the virtual env for ubuntu and sles, we discussed in office hours that we would ideally like to keep one requirements file. In the testing for the required packages, I saw two test cases failing:
1.
It should pick up the library but not sure why it is not picking that up. I was able to make the test case pass by specifying the full path to the library in the test case. 2.
To fix this I installed a more recent version of
If I try to install an older version of If I install If I try to install
TLDR: I cannot get the required version of protobuf to install that will support onnx version clsoe to onnx 1.14.1. |
Updates for the above 2 issues: 2.
There is no version of protobuf for I also tried the other option to use I tried backporting Looks like we would have to disable this test for python3.6. |
Updated PR to add support for python version specific requirement files. We now have a base requirements file, python 3.6 requirement file and a common requirement file for all other python versions. This can be extended to add more python version specific requirement files if they are needed. |
I got feedback on my PR yesterday and I have addressed all the comments. |
Re-opening as the main PR needs to be merged in. |
This PR removed the try-except clause for importing numpy tests because the tests would silently exit if numpy was not found. It is preferable to have the tests fail if the numpy requirement is not satisfied, rather than silently exiting. |
The test cases of "make check" can fail over time due to upstream changes when explicit versions are not captured. This is important for official releases (release/rocm-rel-5.x). The Develop and Master branch should keep up with latest possible to catch issues but need to fix upstream dependencies once in to release branches. An example of this is Protobuf dropping support for Python3.6. Our rocm-rel-5.2 stream now doesn't build all of it's CI because Protobuf was allowed to float. So unit tests now fail on OS' that have python3.6
I'd like to see our build support Virtual Environments to easily pin python packages at to specific configurations
The text was updated successfully, but these errors were encountered: