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

Convert MPT checkpoints to FT format #169

Merged
merged 1 commit into from
May 24, 2023
Merged

Conversation

dskhudia
Copy link
Contributor

A script to convert MPT checkpoints to FT format. A script to run with FasterTransformer is coming soon in a followup PR.

Partially fixes #67

Thanks @bheilbrun for sharing your code.

@dskhudia dskhudia force-pushed the convert_ft branch 3 times, most recently from 362b7c4 to a392dc4 Compare May 19, 2023 00:02
scripts/inference/ft/mpt_ckpt_to_ft.py Outdated Show resolved Hide resolved
scripts/inference/ft/mpt_ckpt_to_ft.py Outdated Show resolved Hide resolved
scripts/inference/ft/mpt_ckpt_to_ft.py Outdated Show resolved Hide resolved
scripts/inference/ft/mpt_ckpt_to_ft.py Outdated Show resolved Hide resolved
scripts/inference/ft/mpt_ckpt_to_ft.py Outdated Show resolved Hide resolved
scripts/inference/ft/mpt_ckpt_to_ft.py Outdated Show resolved Hide resolved
scripts/inference/ft/mpt_ckpt_to_ft.py Outdated Show resolved Hide resolved
scripts/inference/ft/mpt_ckpt_to_ft.py Outdated Show resolved Hide resolved
scripts/inference/ft/mpt_ckpt_to_ft.py Outdated Show resolved Hide resolved
scripts/inference/ft/mpt_ckpt_to_ft.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@hanlint hanlint left a comment

Choose a reason for hiding this comment

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

To avoid nesting too many folders, can we remove the ft folder and just put this in the root directory of inference ?

Also, while this is a script, users may want to call the main conversion function progammatically. Can we rename that method something more clear, e.g. convert_to_faster_transformer and type/document the input variables?

scripts/inference/ft/mpt_ckpt_to_ft.py Outdated Show resolved Hide resolved
scripts/inference/ft/mpt_ckpt_to_ft.py Outdated Show resolved Hide resolved
@dskhudia
Copy link
Contributor Author

To avoid nesting too many folders, can we remove the ft folder and just put this in the root directory of inference ?

@hanlint : I created a folder for two reasons 1) There will be at least one another script so all FT related stuff can stay in a single folder 2) Readme of this folder can explain all the FT related stuff in a single place.

@dskhudia dskhudia force-pushed the convert_ft branch 4 times, most recently from 6a801a6 to cd04727 Compare May 22, 2023 22:27
Copy link
Member

@abhi-mosaic abhi-mosaic left a comment

Choose a reason for hiding this comment

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

Left some small comments, but overall I think this would be great to merge soon to get community feedback. I would also love to have tests in this repo but I understand that the FT deps+installation is quite tedious. So as long as @dskhudia you have tested manually, no need to block on that.

I do agree with @hanlint that it makes sense to move these files up a level to scripts/inference, even if there end up being 2 or 3 scripts for FT. I'd like to have it all in one place until it grows big enough to necessitate a folder.

@dskhudia would it be ok to move this script a folder and append the short README.md here to llm/inference/README.md?

scripts/inference/ft/README.md Outdated Show resolved Hide resolved
scripts/inference/ft/mpt_ckpt_to_ft.py Outdated Show resolved Hide resolved
@dskhudia
Copy link
Contributor Author

would it be ok to move this script a folder and append the short README.md here to llm/inference/README.md

moved it to the inference folder.

Copy link

@bheilbrun bheilbrun left a comment

Choose a reason for hiding this comment

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

Thanks in particular for the code quality improvements!

@dskhudia dskhudia merged commit a7e5209 into mosaicml:main May 24, 2023
6 checks passed
@dskhudia dskhudia deleted the convert_ft branch May 24, 2023 05:43
@dskhudia dskhudia mentioned this pull request May 24, 2023
bmosaicml pushed a commit that referenced this pull request Jun 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FasterTransformer
5 participants