mlc-llm icon indicating copy to clipboard operation
mlc-llm copied to clipboard

[Bug] Relax repository should be a submodule

Open tobrun opened this issue 2 years ago • 3 comments

tldr not having relax repository as a submodule makes older versions of this repository hard to build.


We currently require the relax repository to be installed through a git clone:

git clone https://github.com/mlc-ai/relax.git --recursive
cd relax
mkdir build
cp cmake/config.cmake build

This however isn't ideal since we always pull in the latest from that repository when setting up the project. If you would go back through the git history, you will be unable to compile because of mismatched version or it would be extremely hard to identify which commit was compatible. The root cause is that there is no direct link between the version of relax versus the code in this repository. A git submodule will solve that issue for us, it allows us to track the versions correctly and will allow us to build older versions of the code more effectively.

A potential additional benefit is that we can simplify installation instructions, since we will know where the relax project will be located in our project path and thus eliminating the usage of the following env variables:

export TVM_HOME=$(pwd)
export PYTHONPATH=$PYTHONPATH:$TVM_HOME/python

tobrun avatar Jun 09 '23 08:06 tobrun

We had discussions on this before and part of the reason we didn't make it a submodule is to avoid duplication (people have already cloned relax and in that case, they only need to set TVM_HOME to that path.

But I totally with you that making it a submodule can simplify installation and track commit id, etc. Currently, we encourage users to run scripts/prep_deps.sh to prepare dependencies.

@tqchen might have more to say on this.

yzh119 avatar Jun 09 '23 09:06 yzh119

I feel your sentiment, I have duplicated codebases across my workspace and often need to push in one project and pull it later in another project submodule. That is the standard in the industry and just a limitation about how git works. I'm guessing this is mostly about convenience and not about size since the models we build with this project are n-times bigger consumer of disk space.

This said, having a non-versioned dependency is a problem in many ways:

  • imagine someone making a breaking change in Relax, new people in setting up this project won't be able to build until it is fixed or they need to go figure out themselves that they need to revert a commit..
  • atm, we don't (yet?) have a CI system, without versioned dependency that would easily break it
  • we are unable to go back in git history time, I tried building the Android integration when it just landed but wasn't able to since I didn't know which version of Relax to use.

I really hope you reconsider your decision on this, the convenience does not out weight the problems it produces.

There are alternative to submodules (like monorepo with gitsubtree) or using actual release that are semver compliant etc. but for this project, with this scope of the dependency, a submodule would be the best fit.

tobrun avatar Jun 09 '23 11:06 tobrun

Thinking this a bit more, I now think having submodule is a good idea. Thanks @tobrun for suggestion

tqchen avatar Jun 09 '23 14:06 tqchen

Hi @tobrun already merged, thanks for your suggestions.

yzh119 avatar Jun 09 '23 23:06 yzh119