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

Updates the export for onnx script to work with HF models #283

Merged
merged 10 commits into from
Apr 7, 2023

Conversation

dakinggg
Copy link
Collaborator

@dakinggg dakinggg commented Apr 6, 2023

  • updates the onnx export script to work directly with HF models
  • fixes some onnx incompatibilities in mosaicgpt
  • adds an onnx export unit test

1b training on this pr: https://wandb.ai/mosaic-ml/daniel-debug/runs/3rnsjn09?workspace=user-danielking
pytest passes: ========================================================== 145 passed, 3 skipped, 31 xfailed, 352 warnings in 311.60s (0:05:11) ===========================================================

@dakinggg dakinggg changed the base branch from main to release/v0.0.4 April 6, 2023 08:18
@dakinggg dakinggg requested review from vchiley, abhi-mosaic and dskhudia and removed request for vchiley April 6, 2023 20:50
@dakinggg dakinggg marked this pull request as ready for review April 6, 2023 20:50
@dskhudia
Copy link
Contributor

dskhudia commented Apr 6, 2023

Would the onnx model work with kv caching? It's not returning past_key_values.

@dakinggg
Copy link
Collaborator Author

dakinggg commented Apr 6, 2023

No, I don't think this works with kv caching right now. I think HF has some support for doing this with https://huggingface.co/docs/transformers/v4.27.2/en/main_classes/onnx#transformers.onnx.OnnxConfigWithPast, but saw the point of this PR more as making sure that our model ops are able to be exported.

@dakinggg dakinggg merged commit 3fb17e9 into mosaicml:release/v0.0.4 Apr 7, 2023
@dakinggg dakinggg deleted the onnx branch June 1, 2023 00:15
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.

2 participants