Skip to content
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

Added mamba model support and test CI script #1573

Open
wants to merge 25 commits into
base: main
Choose a base branch
from

Conversation

zzhang37
Copy link

@zzhang37 zzhang37 commented Dec 6, 2024

What does this PR do?

Added Mamba model support using custom op, and added the test on Mamba, the test command is
GAUDI2_CI=1 RUN_SLOW=1 python -m pytest tests/test_text_generation_example.py -s -v -k "mamba"
It passed on 1.18.0release and 1.19.0, build 497 with corresponding so files.

Fixes # (issue)

Before submitting

  • This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • Did you make sure to update the documentation with your changes?
  • Did you write any new necessary tests?

@zzhang37 zzhang37 requested a review from regisss as a code owner December 6, 2024 21:45
@zzhang37
Copy link
Author

zzhang37 commented Dec 6, 2024

Just run the command
GAUDI2_CI=1 RUN_SLOW=1 python -m pytest tests/test_text_generation_example.py -s -v -k "mamba"
It will pass on 1.18 and 1.19 version

setup.py Outdated Show resolved Hide resolved
Comment on lines 213 to 217
```bash
wget https://huggingface.co/Habana/mamba/resolve/main/hpu_custom_pscan_all.cpython-310-x86_64-linux-gnu_119.so
wget https://huggingface.co/Habana/mamba/resolve/main/libcustom_tpc_perf_lib_119.so
export HABANA_CUSTOM_OP_DIR=/path/of/hpu_custom_pscan_all.cpython-310-x86_64-linux-gnu_119.so located
export GC_KERNEL_PATH=/path/to/libcustom_tpc_perf_lib_119.so:$GC_KERNEL_PATH
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be done automatically in the script if the model type is Mamba

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test did the automatically download, but for users want to run the mamba, they can just download the so files form the Habana/mamba card.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also remove the wget and use hf_hub_download in the test script.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this will discourage many users to use this model. Can't we put the code you added to the test file to donwload the .so in a new utils file in the Mamba modeling folder? And then we could call it here and in the test.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just readme file.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what you mean. I think automatically downloading these .so files in the script if the model type is mamba would give a better user experience. Why not doing it there?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zzhang37 it seems the mamba run code fails if we don't set this environment variable at all. Is this intended? Can customer run with/without custom code? If the intention is always run mamba with custom kernel, we should put all these download/setup code in modeling_mamba file at the beginning before you load the library.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@regisss Removed the all wget and added util to do the job.
@jiminha Just checked with latest build 538, works fine.

README.md Show resolved Hide resolved
Copy link

github-actions bot commented Dec 9, 2024

The code quality check failed, please run make style.

@HuggingFaceDocBuilderDev

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

@zzhang37
Copy link
Author

Fixed the make style issue.

@zzhang37 zzhang37 requested a review from mandy-li as a code owner December 11, 2024 19:35
@libinta libinta added the run-test Run CI for PRs from external contributors label Dec 12, 2024
@zzhang37 zzhang37 requested a review from libinta as a code owner December 12, 2024 17:43
@regisss
Copy link
Collaborator

regisss commented Dec 12, 2024

@zzhang37 There are some new issues now but these should be quite straightforward to fix:

  • Calling adapt_transformers_to_gaudi will return an error if GC_KERNEL_PATH has not been specified before. I suggest to run this piece of code only if GC_KERNEL_PATH is in os.environ. If I understood correctly, GC_KERNEL_PATH should be specified if the user intends to use Mamba right?
  • The test doesn't work because GC_KERNEL_PATH is not specified. You should specify it similarly to how it is done here if the tested model is Mamba.

@zzhang37
Copy link
Author

zzhang37 commented Dec 12, 2024

@regisss GC_KERNEL_PATH is used by all models. There is a default path for GC_KERNEL_PATH, but mamba required an additional path to add .The problem is GC_KERNEL_PATH needs to be set before the start of the script since Synapse needs to use this path to load the TPC kernels in the beginning, if we add additional path to GC_KERNEL_PATH in the middle of the script, it will miss the loading of the additional kernels we added in the path, the mamba will not able to use that kernel. So that is the reason if we run mamba model, we need to add additional path to GC_KERNEL_PATH beforehand rather than in the middle of the script. Just like command we should use is
C_KERNEL_PATH=/root/.cache/huggingface/hub/models--Habana--mamba/blobs/libcustom_tpc_perf_lib.so:$GC_KERNEL_PATH python run_generation.py .....

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
run-test Run CI for PRs from external contributors
Projects
None yet
Development

Successfully merging this pull request may close these issues.