[GPTQ] fix imports of moved add ons

[yannicks1]

[GPTQ] fails to start with upstream aiu-fms repository

closes #42

changes:

• fix import in model loader (here)

Tested successfully on AIU Spyre with granite 8b.

[github-actions]

👋 Hi! Thank you for contributing to vLLM support on Spyre.
Just a reminder: Make sure that your code passes all the linting checks, otherwise your PR won't be able to be merged. To do so, first install the linting requirements, then run format.sh and commit the changes:

pip install -r requirements-lint.txt
bash format.sh

Now you are good to go 🚀

[tdoublep]

Should we perhaps just raise an exception here (since we don't expect it to work)?

[yannicks1]

I inserted a logger warning. This way it does not directly raise an exception if this should work in the future due to some changes in the fms repos.

[tdoublep]

If it doesn't work now, I would still prefer to raise an exception rather than warning if it is unsupported configuration. Exception is much better feedback to the user.

[joerunde]

Is the above sys.path.append("/home/senuser/aiu-fms") still required now that the fms_mo package exists? It's not clear from the comment what we're actually importing from that path

[joerunde]

Also, we should ideally be tracking fms_mo as a dependency, but I'm assuming that it's not published to pypi yet, right? Ah JK I see it is released here, though it looks like there isn't much activity on getting new releases pushed: https://pypi.org/project/fms-model-optimizer/#history

[dpatel-ops]

I have not tried gptq but yes I agree we may not need aiu-fms anymore as everything there should be either in fms_mo or in fms.

[tdoublep]

Agree we need to add fms_mo as a dep.

[yannicks1]

good point, I could not find any other dependencies on aiu-fms and thus removed it.

[yannicks1]

I am struggling to add fms_mo as a dependency in requirements.txt (see my last two commits). Does this have to happen as part of this PR. If yes, can you possibly help @dpatel-ops ?

[joerunde]

@yannicks1 it looks like fms-mo only supports python <=3.11, but our build uses python 3.12

We could downgrade our build to 3.11, or work with the fms-mo team to build for 3.12 as well. Usually that's a simple build config setting, but I don't know enough to know if fms-mo specifically has problems supporting 3.12.

I would prefer to get a release of fms-mo supporting 3.12, but if we need to get this change in with some README updates that mention gptq support is only on py311 that would be okay as well.

[joerunde]

Shoot, it looks like with torch 2.5.1 the install of triton might brick model loading with vllm 0.8.2 :(

[tjohnson31415]

Shoot, it looks like with torch 2.5.1 the install of triton might brick model loading with vllm 0.8.2 :(

I reset my dev environment and installed vllm 0.8.2 with fms_mo (hacked to install with Python 3.12, also had to uninstall torchvision) and it doesn't raise the TypeError: must be called with a dataclass type or instance error. fms_mo pins triton < 3.2 and it is actually triton==3.2 with torch==2.5.1 that seem to be incompatible.

I don't know enough to know if fms-mo specifically has problems supporting 3.12.

I did see a note in the README:

📋 Python 3.12 is currently not supported due to PyTorch Dynamo constraint

Though that may be an out-of-date note. This PyTorch issue indicates that it was made compatible in PyTorch 2.4.

[joerunde]

Though that may be an out-of-date note. pytorch/pytorch#120233 (comment) indicates that it was made compatible in PyTorch 2.4.

Interesting, I had actually assumed that some part of fms-mo that used dynamo required updating based on changes in python 3.12. But you know what happens when you assume lol

@yannicks1 I opened an issue for handling dependencies here: #61

If you want to, I think I'd be fine merging this in without the dependencies specified, but I'll leave it up to you.

[joerunde]

LGTM given that we have issues open to add tests and specify dependencies correctly. Looks like it'll be a bit more work to get that all set up, I'm fine if we merge first and follow up with those

[yannicks1]

Thanks @joerunde and @tjohnson31415 for digging a bit deeper here. Looks like a non-trivial thing to solve, will merge this and we deal with the dependencies in this issue #61 (thanks for opening this Joe).