[razvcris]

Summary

Replaces PR #214, rebased with the depending PR #233 . Original work by @jinsun-yoo

• Renames the package to mlc-chakra (pip install mlc-chakra)
• Shows an install message instead of ModuleNotFoundError traceback when et_replay is missing
• Updates USER_GUIDE.md, requirements-dev.txt and src/trace_link/trace_link.py
• Conflicts with Update HTA dependency to the traceinsight package #233 were resolved

Test Plan

Environment: Python 3.11.18 venv, et_replay installed from PARAM

• pip install .: Successfully installed mlc-chakra-1.0.0
• python -m pytest tests: All 117 tests pass
• chakra_trace_link without et_replay installed: Prints the install message from et_replay_import_error.py, with no traceback
• chakra_trace_link with et_replay installed: Works normally

Closes #239

[github-actions]

MLCommons CLA bot All contributors have signed the MLCommons CLA ✍️ ✅

[winstonliu1111]

@briancoutinho please (re-) assign additional reviewer as appropriate, I added you for now.

[winstonliu1111]

any idea why there is a dependency to et_replay (when theoretically, that should be depending on chakra instead)....i understand this may be due to historical reasons, just wondering.

[theodorbadea]

Please rebase this on the latest main and give it a proper name, with a clear reference to what the PR is doing, e.g. Changes for pypi packaging, as the original PR was named.

[theodorbadea]

any idea why there is a dependency to et_replay (when theoretically, that should be depending on chakra instead)....i understand this may be due to historical reasons, just wondering.

IMO, Chakra needs to own the tree building mechanics, so I believe we should internalize the stuff used from et_replay and remove the dependency. Anyway et_replay is not on pypi so we would get rid of a pip install impediment too. @mihncalo already looks into this.

No replay part is actually used from et_replay, only the trace parser during trace linking. load_execution_trace_file parses the host json trace and the et_replay Node datastructure is used to model the PyTorchOperator. Moreover, the chakra convertor already has some sort of parser so we can probably build on that.

[briancoutinho]

@theodorbadea
Actually param is also not being maintained anymore and the replay got moved to repo in meta-pytorch:
https://github.com/meta-pytorch/chakra_replay/blob/main/et_replay/execution_trace.py

Anyways I agree we can duplicate and move the execution_trace.py file here. I think that is all you need :)

[theodorbadea]

1. This will raise an ImportError and then I think the console will print the full traceback with the hint probably barely visible at the bottom. Also I think the guards are mostly unreachable and perhaps quite polluting in 6 places. And from cli i think only the one in trace_linker.py is reachable. I believe a single check in main() of trace_link.py would solve the issue and make the message clear too, e.g.:

args = parser.parse_args()
if importlib.util.find_spec("et_replay") is None:
    sys.exit(get_et_replay_install_error_msg())
from .trace_linker import TraceLinker

String checking like "et_replay" in str(e) verifies by text, so it also matches things like cannot import name … from 'et_replay.utils' (/…/et_replay/utils.py) (for example when there is an installed but mismatched et_replay) and even unrelated import errors whose file path contains et_replay. Moreover when combined with from None, the user is told to install something they already have and loses the real cause. find_spec("et_replay") is None tests the presence directly.
2. pin the versions for build and twine please
3. import chakra is unchanged, so someone with chakra installed from source and then pip install mlc-chakra ends up with two distributions of the same files. You also removed pip uninstall chakra from the guide, so a one-line note like "uninstall the source-installed version first" note would help when someone will use pip install.

4. Lets truly respect the naming, so please correct "PyPi" with "PyPI".

[theodorbadea]

Please update the PR description becasue now "tests/trace_link" is no longer modified and the test plan still says "shows the ImporError...".