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 new model support Cohere/Command-R #154

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

Conversation

quic-akuruvil
Copy link
Contributor

@quic-akuruvil quic-akuruvil commented Oct 14, 2024

  1. Added the model architecture changes.
  2. Added a new input parameter: inputs_embeds instead of input_ids to calculate embeddings in host.
  3. Adjusted the code flow to integrate the new input - inputs_embeds
  4. CB mode enabled for this

@quic-akuruvil quic-akuruvil self-assigned this Oct 14, 2024
@quic-akuruvil quic-akuruvil added wip Work in progress enhancement New feature or request and removed wip Work in progress labels Oct 14, 2024
#
# Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
# SPDX-License-Identifier: BSD-3-Clause
#
Copy link
Contributor

Choose a reason for hiding this comment

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

can you add a delimiter here?


Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure

@anujgupt-github
Copy link
Contributor

@anujgupt-github
Copy link
Contributor

@quic-akuruvil can you please add accuracy evaluation run details?

@quic-akuruvil
Copy link
Contributor Author

@quic-akuruvil can you please add accuracy evaluation run details?

Yes, I tried running the perplexity script, but it broke. I’ll look into it and get the numbers.

@@ -72,6 +72,7 @@ def main(
cache_dir=cache_dir,
hf_token=hf_token,
)
embeds, config = get_embeddings(model_name, hf_token, cache_dir, local_model_dir)
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to generalize this? I don't this is accessible for all the model categories.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes right, we can make this conditional, fetch embeddings only for Cohere.

@@ -204,18 +204,23 @@ def export_kvstyle_transformed_model_to_onnx(
raise ValueError(f"Need seq_len to be greater than zero, got seq_len={seq_len}")

# Preprocess inputs
embeds = None
if model_name == "CohereForAI/c4ai-command-r-v01":
Copy link
Contributor

Choose a reason for hiding this comment

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

If we want to do this for all the versions of cohere and not specific to this model_name, then we need to do it as torch level? not a good practice to do at model_name level

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay sure, I have changed it to those architecture with CohereCausalLM head in architecture, so it's more generic.

@quic-akuruvil
Copy link
Contributor Author

quic-akuruvil commented Nov 5, 2024

@quic-akuruvil can you please add accuracy evaluation run details?

image
image

The perplexity is matching for onnx and qpc.

@quic-akuruvil
Copy link
Contributor Author

@quic-akuruvil can you please add accuracy evaluation run details?

image image

The perplexity is matching for onnx and qpc.

for CL=1024, perplexity results are as below:
image
image

@quic-rishinr quic-rishinr added the in-review Review process is ongoing label Nov 6, 2024
@quic-akuruvil
Copy link
Contributor Author

@quic-akuruvil can you please add accuracy evaluation run details?

Done

@quic-rishinr
Copy link
Contributor

@quic-akuruvil Can you please resolve the conflicts?

@quic-akuruvil quic-akuruvil added the wip Work in progress label Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request in-review Review process is ongoing wip Work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants