Add support for cohere/titan embedding models

[QuinnGT]

Issue #, if available:

Issues open that this resolves, #218

Description of changes:

Brought over changes from spugachev 3 month old branch on cohere. Added changes for new schema. Deployed and tested in fresh environment last night but I see massi-ang opened #359 this morning. However cohere embedding needs additional changes for how it handles inputs. Documented here.

search_document type and search_query type are used. Classification and clustering are not needed at this time.

embedding_types was not defined as float is appropriate for this implementation.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[bigadsoleiman]

Thanks for this submission!

Just added a couple of feedbacks

[massi-ang]

Thanks for the PR.

[QuinnGT]

Quick update. I pushed updates for everything except for titan multimodal. Currently deploying those changes and testing before taking on titan multimodal.

In regards to Titan Multimodal Embeddings, it will need a bit more than just calling out the additional model. It will need support for it's API payload like max input text tokens and max input image size. However we are not capturing images in our use of Unstructured or rather lack of use of unstructured, like the partition_image function. Also I see unstructured is poorly used as we pull the full container yet only use the loader.

As soon as the cohere changes finish and tests pass, I'll circle back on titan multimodal and unstructured. Feel free to post thoughts or feedback.

[massi-ang]

I uderstand your concerns, but I think they are out of scope to just adding this embedding model for text embeddings. maxInputTokens is optional and we currently only need to support text embeddings. This was already tested in #359 and working fine.

We should then work to better leverage the multimodal embeddings, but that is an unrelated feature to this one.

[QuinnGT]

Alright, all requested changes have been made. I also fixed the embedding prompt to show when enableRag was true vs just being skipped no matter what the use selected, in the magic-config.

New environment has been deployed. Bedrock and Sagemaker embedding models tested as functional.

[QuinnGT]

Found issue with creating workspace. Not sure if it's from me or changes merged into this PR. Need to test and see.

[QuinnGT]

Resolved the latest issues. Feel free to test and let me know if anything else is needed.

[QuinnGT]

@bigadsoleiman I noticed an error in deployment that's happening for log groups due to size of name. I tested it here and it works. Do you want me to bring that into this PR or wait till this is approved?

[massi-ang]

@bigadsoleiman I noticed an error in deployment that's happening for log groups due to size of name. I tested it here and it works. Do you want me to bring that into this PR or wait till this is approved?

Hi Quinn, I checked your change and my concern is that if you deploy two stacks, they will share the same log group, since the value of id will be the same in for both.

[QuinnGT]

@bigadsoleiman I noticed an error in deployment that's happening for log groups due to size of name. I tested it here and it works. Do you want me to bring that into this PR or wait till this is approved?

Hi Quinn, I checked your change and my concern is that if you deploy two stacks, they will share the same log group, since the value of id will be the same in for both.

Great point @massi-ang . How about I use the config.prefix with the id so it looks something like logGroupName: /aws/vendedlogs/states/${config.prefix}-${id}.

Before: /aws/vendedlogs/states/CreateAuroraWorkspace-CreateAuroraWorkspaceSMLogGroup
After: /aws/vendedlogs/states/dev-CreateAuroraWorkspaceSMLogGroup

[massi-ang]

I would suggest using /aws/vendedlogs/states/${cdk.Stack.of(this).name}-${id}, but there could still be issues in case we apply the same solution to other constructs which have the same id (since id in only unique in the scope of the parent construct).

[QuinnGT]

@bigadsoleiman is there anything else pending that I missed? If not, can you approve this PR? Thank you!

[Cerrix]

Hi @QuinnGT,
I tested the pull request because I was looking for the Cohere Multilingual implementation. When I selected that model for embeddings, I received the following error response from AppSync:
{ "data": null, "errors": [ { "path": [ "calculateEmbeddings" ], "data": null, "errorType": "TypeError", "errorInfo": null, "locations": [ { "line": 2, "column": 3, "sourceName": null } ], "message": "Object of type Task is not JSON serializable" } ] }
both creating the workspace or testing the embedding.
With Titan everything is fine.

Thank you!

[QuinnGT]

Hi @QuinnGT, I tested the pull request because I was looking for the Cohere Multilingual implementation. When I selected that model for embeddings, I received the following error response from AppSync: { "data": null, "errors": [ { "path": [ "calculateEmbeddings" ], "data": null, "errorType": "TypeError", "errorInfo": null, "locations": [ { "line": 2, "column": 3, "sourceName": null } ], "message": "Object of type Task is not JSON serializable" } ] } both creating the workspace or testing the embedding. With Titan everything is fine.

Thank you!

@Cerrix thank you for reporting this. Can you share what RAG you were testing with?

[Cerrix]

Yep: I’m testing in with Open Search (and just that). I get the same error even if testing cohere through the “embeddings” page (not only creating a new workspace).

thank you

[QuinnGT]

Yep: I’m testing in with Open Search (and just that). I get the same error even if testing cohere through the “embeddings” page (not only creating a new workspace).

thank you

Hey @Cerrix I enabled and deployed an opensearch instance and I'm not seeing any errors with workspace creation or testing embeddings. I'm not sure if it's a new or existing deployment you are testing in but try doing a fresh deployment from the latest, along with a new npm install and build. Feel free to report back with more information to troubleshoot, if needed.

[QuinnGT]

Comparison of embedding models from demo data.

[Cerrix]

It is a week old deploy. But let me try again from scratch! I'll keep you posted!

[Cerrix]

It is so strange. I re-deployed from scratch the repository branching your PR in Oregon (us-west-2).

Here is my config file:

{
  "prefix": "llmchatbot-demo",
  "privateWebsite": false,
  "certificate": "",
  "domain": "",
  "bedrock": {
    "enabled": true,
    "region": "us-west-2"
  },
  "llms": {
    "sagemaker": []
  },
  "rag": {
    "enabled": true,
    "engines": {
      "aurora": {
        "enabled": false
      },
      "opensearch": {
        "enabled": true
      },
      "kendra": {
        "enabled": false,
        "createIndex": false,
        "external": [],
        "enterprise": false
      }
    },
    "embeddingsModels": [
      {
        "provider": "sagemaker",
        "name": "intfloat/multilingual-e5-large",
        "dimensions": 1024
      },
      {
        "provider": "sagemaker",
        "name": "sentence-transformers/all-MiniLM-L6-v2",
        "dimensions": 384
      },
      {
        "provider": "bedrock",
        "name": "amazon.titan-embed-text-v1",
        "dimensions": 1536
      },
      {
        "provider": "bedrock",
        "name": "amazon.titan-embed-image-v1",
        "dimensions": 1024
      },
      {
        "provider": "bedrock",
        "name": "cohere.embed-english-v3",
        "dimensions": 1024
      },
      {
        "provider": "bedrock",
        "name": "cohere.embed-multilingual-v3",
        "dimensions": 1024,
        "default": true
      },
      {
        "provider": "openai",
        "name": "text-embedding-ada-002",
        "dimensions": 1536
      }
    ],
    "crossEncoderModels": [
      {
        "provider": "sagemaker",
        "name": "cross-encoder/ms-marco-MiniLM-L-12-v2",
        "default": true
      }
    ]
  }
}

Still I got the same error:

[QuinnGT]

@Cerrix Interesting, I'm not sure. It looks like it's an error from the appsync function. Almost like the old version of app sync functions are trying to be used with the newer genai_core python functions. Did you purge your cdk.out when doing a fresh deployment? Also double check cdk version is updated on your local as there seems to be some rebuild bug introduced on each release.

@massi-ang I see Bigad approved the PR but we're still waiting on you or another repo owner for approval. What else do you need to from me?

[Cerrix]

I'm deploying following the Cloud9 instructions of the sample documentation: https://aws-samples.github.io/aws-genai-llm-chatbot/guide/deploy.html

If we need to update some dependencies I think it must be documented before merging the PR in main.

I can try with a vanilla cloud9 instance but we need to double check the issue. I don't know if someone from the team would like to try the deploy!

[massi-ang]

Please fix the Provider type with the right semantics