[Hexagon] Improved ergonomics of HexagonLauncher in unit tests. #10581
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Looks like the current failures in CI are due to the The |
mehrdadh
reviewed
Mar 17, 2022
| return port | ||
|
|
||
|
|
||
| @pytest.fixture(scope="session") | ||
| def tvm_tracker(tvm_tracker_port): |
There was a problem hiding this comment.
In nightly test we assume there's a tvm_tracker already running with registered devices on it. So I suggest either remove tvm_tracker from pytext feature or if you could do something more complicated that like this it should work. You can query the server/port and if there was a response don't instantiate the tracker.
There was a problem hiding this comment.
Thank you, and I hadn't accounted for that. It looks like it is handled similarly in tests/scripts/task_python_hexagon_simulator.sh, with the script starting the tracker before running pytest. I still like having the tracker start from the pytest call, since that minimizes the amount of local setup needed to reproduce a CI failure, but I can see how it would be useful to avoid repeated setup/teardown of the tracker in CI.
I like the idea of choosing whether pytest starts a tracker, though I think I'll implement it based on the presence of the TVM_TRACKER_HOST and TVM_TRACKER_PORT environment variables. This would avoid a potential failure mode from using a server query in a multi-user system, where somebody else's tracker is mistaken for one's own.
The goal of this commit is to reduce/eliminate common code required
through unit tests that interact with Hexagon hardware.
- New testing fixtures in `tests/python/contrib/test_hexagon`. A test
running on hexagon hardware should only need to use the
`hexagon_session` fixture.
- `rpc_server_port`: Iterates through port numbers, selecting an
unused port for each unit test. Avoids needing to explicitly
specify unique ports for each unit test.
- `tvm_tracker`: Starts a tracker on use, exits after test. Avoids
needing to manually start a tracker prior to running the unit
test.
- `hexagon_launcher`: Starts a `HexagonLauncher` server on use,
stops server after test. Avoids needing to call `start_server()`
and `stop_server()` in each test.
- `hexagon_session`: Starts a hexagon session using
`hexagon_laucnehr.start_session()`, exits after test.
- Added `Session.upload` function, which delegates to
`HexagonLauncher.upload`. Avoids needing to interact with both the
launcher and the session.
- Allowed `tvm.IRModule` as argument passed to `Session.load_module`,
which will automatically save/upload the module, then load it.
Avoids needing to handle save/upload of temporary files in each unit
test.
Lunderberg
force-pushed
the
hexagon_launcher_ergonomics
branch
from
b73501a to
9c16f66
Compare
|
Rebased onto main to resolve merge conflict. |
Lunderberg
force-pushed
the
hexagon_launcher_ergonomics
branch
from
9c16f66 to
52fda7e
Compare
csullivan
reviewed
Mar 24, 2022
csullivan
approved these changes
Mar 24, 2022
…vm into hexagon_launcher_ergonomics
Lunderberg
force-pushed
the
hexagon_launcher_ergonomics
branch
from
a611b3c to
f7844ef
Compare
|
Looks like the Windows build failed due to network failure, all other tests passed. Pushed a dummy commit to restart the CI. |
Lunderberg
added a commit
to Lunderberg/tvm
that referenced
this pull request
Apr 5, 2022
Follow-up from apache#10581, applying similar changes to the AOT and graph executor interactions. This moves the file management and upload/download from the unit tests into the launcher.
Lunderberg
added a commit
to Lunderberg/tvm
that referenced
this pull request
Apr 7, 2022
Should have been removed as part of apache#10581.
Lunderberg
added a commit
to Lunderberg/tvm
that referenced
this pull request
Apr 8, 2022
Should have been removed as part of apache#10581.
kparzysz-quic
pushed a commit
that referenced
this pull request
Apr 8, 2022
Should have been removed as part of #10581.
pfk-beta
pushed a commit
to pfk-beta/tvm
that referenced
this pull request
Apr 11, 2022
…he#10581) * [Hexagon] Improved ergonomics of HexagonLauncher in unit tests. The goal of this commit is to reduce/eliminate common code required through unit tests that interact with Hexagon hardware. - New testing fixtures in `tests/python/contrib/test_hexagon`. A test running on hexagon hardware should only need to use the `hexagon_session` fixture. - `rpc_server_port`: Iterates through port numbers, selecting an unused port for each unit test. Avoids needing to explicitly specify unique ports for each unit test. - `tvm_tracker`: Starts a tracker on use, exits after test. Avoids needing to manually start a tracker prior to running the unit test. - `hexagon_launcher`: Starts a `HexagonLauncher` server on use, stops server after test. Avoids needing to call `start_server()` and `stop_server()` in each test. - `hexagon_session`: Starts a hexagon session using `hexagon_laucnehr.start_session()`, exits after test. - Added `Session.upload` function, which delegates to `HexagonLauncher.upload`. Avoids needing to interact with both the launcher and the session. - Allowed `tvm.IRModule` as argument passed to `Session.load_module`, which will automatically save/upload the module, then load it. Avoids needing to handle save/upload of temporary files in each unit test. * Added default port for tracker if not already set. * Pass through None from hexagon_launcher to hexagon_session. * Updated launcher to use external tracker if specified. * Avoid setting up the local tracker unless required. * Declare previous_port as global, instead of list. * Corrected type hints. * Docstring updates
mehrdadh
pushed a commit
to mehrdadh/tvm
that referenced
this pull request
Apr 11, 2022
Should have been removed as part of apache#10581.
kparzysz-quic
pushed a commit
that referenced
this pull request
Apr 12, 2022
* [Hexagon] Move aot/graph_executor interactions into launcher Follow-up from #10581, applying similar changes to the AOT and graph executor interactions. This moves the file management and upload/download from the unit tests into the launcher. * Added Session.test_executor to avoid duplication in graph/aot test. * Resolve lint errors * Moved link flags workaround out of session, into create_aot_shared * Separated Session.get_*_executor and Session.get_executor_from_factory * Updated to resolve lint error
Lucien0
pushed a commit
to Lucien0/tvm
that referenced
this pull request
Apr 19, 2022
Should have been removed as part of apache#10581.
Lucien0
pushed a commit
to Lucien0/tvm
that referenced
this pull request
Apr 19, 2022
…10907) * [Hexagon] Move aot/graph_executor interactions into launcher Follow-up from apache#10581, applying similar changes to the AOT and graph executor interactions. This moves the file management and upload/download from the unit tests into the launcher. * Added Session.test_executor to avoid duplication in graph/aot test. * Resolve lint errors * Moved link flags workaround out of session, into create_aot_shared * Separated Session.get_*_executor and Session.get_executor_from_factory * Updated to resolve lint error
altanh
pushed a commit
to altanh/tvm
that referenced
this pull request
Apr 28, 2022
Should have been removed as part of apache#10581.
altanh
pushed a commit
to altanh/tvm
that referenced
this pull request
Apr 28, 2022
…10907) * [Hexagon] Move aot/graph_executor interactions into launcher Follow-up from apache#10581, applying similar changes to the AOT and graph executor interactions. This moves the file management and upload/download from the unit tests into the launcher. * Added Session.test_executor to avoid duplication in graph/aot test. * Resolve lint errors * Moved link flags workaround out of session, into create_aot_shared * Separated Session.get_*_executor and Session.get_executor_from_factory * Updated to resolve lint error
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The goal of this commit is to reduce/eliminate common code required through unit tests that interact with Hexagon hardware.
New testing fixtures in
tests/python/contrib/test_hexagon. A test running on hexagon hardware should only need to use thehexagon_sessionfixture.rpc_server_port: Iterates through port numbers, selecting an unused port for each unit test. Avoids needing to explicitly specify unique ports for each unit test.tvm_tracker: Starts a tracker on use, exits after test. Avoids needing to manually start a tracker prior to running the unit test.hexagon_launcher: Starts aHexagonLauncherserver on use, stops server after test. Avoids needing to callstart_server()andstop_server()in each test.hexagon_session: Starts a hexagon session usinghexagon_launcher.start_session(), exits after test.Added
Session.uploadfunction, which delegates toHexagonLauncher.upload. Avoids needing to interact with both the launcher and the session.Allowed
tvm.IRModuleas argument passed toSession.load_module, which will automatically save/upload the module, then load it. Avoids needing to handle save/upload of temporary files in each unit test.