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.

Sign up for GitHub

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

Jump to bottom

Default to stream for HF model parsers #867

Closed
wants to merge 3 commits into from
Closed

Default to stream for HF model parsers #867

wants to merge 3 commits into from

Conversation

saqadri
Copy link
Contributor

@saqadri saqadri commented Jan 10, 2024 •
edited
Loading

Default to stream for HF model parsers

Default stream to True if no inference options are specified.

This is the expected behavior.

Test Plan:

  • Tried model parsers without this change --> no streaming

  • After this change --> streaming for the same aiconfig

Stack created with Sapling. Best reviewed with ReviewStack.

@saqadri
Support custom specified models for HF model parsers
75d3a2d 
The HF prompt schema has been updated in #850 to support a "model" property, which can specify the model to use for a given HF task.

This change updates the model parsers to use this setting.

Test Plan:

Tested with the following prompt:
```
    {
      "name": "image_caption",
      "input": {
        "attachments": [
          {
            "data": "https://s3.amazonaws.com/lastmileai.aiconfig.public/uploads/2024_1_10_18_41_31/1742/bear_with_honey.png",
            "mime_type": "image/png"
          },
          {
            "data": "https://s3.amazonaws.com/lastmileai.aiconfig.public/uploads/2024_1_10_18_41_31/7275/fox_in_forest.png",
            "mime_type": "image/png"
          }
        ]
      },
      "metadata": {
        "model": {
          "name": "HuggingFaceImage2TextTransformer",
          "settings": {
            "max_new_tokens": 10,
            "model": "Salesforce/blip-image-captioning-base"
          }
        },
        "parameters": {}
      },
      "outputs": [
        {
          "output_type": "execute_result",
          "execution_count": 0,
          "data": "a bear sitting on a rock eating honey",
          "metadata": {}
        },
        {
          "output_type": "execute_result",
          "execution_count": 1,
          "data": "a red fox in the woods",
          "metadata": {}
        }
      ]
    },
```

Validated the the model setting was respected and worked.
This was referenced Jan 10, 2024
Merged
Merged
Merged
@saqadri
Fix parameterization issues
18defd8 
Now that we have non-text input model parsers, we can see some issues in our parameterization.

1) ASR and Image-to-Text modelparsers should NOT be `ParameterizedModelParser` instances, since their inputs cannot be parameterized.

2) Parameter resolution logic shouldn't throw errors in the case where it's parsing prompts belonging to regular model parsers.

Test Plan:
Ran the translation prompt for this aiconfig:

```
prompts: [
{
      "name": "image_caption",
      "input": {
        "attachments": [
          {
            "data": "https://s3.amazonaws.com/lastmileai.aiconfig.public/uploads/2024_1_10_18_41_31/1742/bear_with_honey.png",
            "mime_type": "image/png"
          },
          {
            "data": "https://s3.amazonaws.com/lastmileai.aiconfig.public/uploads/2024_1_10_18_41_31/7275/fox_in_forest.png",
            "mime_type": "image/png"
          }
        ]
      },
      "metadata": {
        "model": {
          "name": "HuggingFaceImage2TextTransformer",
          "settings": {
            "max_new_tokens": 10,
            "model": "Salesforce/blip-image-captioning-base"
          }
        },
        "parameters": {}
      },
      "outputs": [
        {
          "output_type": "execute_result",
          "execution_count": 0,
          "data": "a bear sitting on a rock eating honey",
          "metadata": {}
        },
        {
          "output_type": "execute_result",
          "execution_count": 1,
          "data": "a red fox in the woods",
          "metadata": {}
        }
      ]
    },
    {
      "name": "translation",
      "input": "Once upon a time, in a lush and vibrant forest, there lived a magnificent creature known as the Quick Brown Fox. This fox was unlike any other, possessing incredible speed and agility that awed all the animals in the forest. With its fur as golden as the sun and its eyes as sharp as emeralds, the Quick Brown Fox was admired by everyone, from the tiniest hummingbird to the mightiest bear. The fox had a kind heart and would often lend a helping paw to those in need. The Quick Brown Fox had a particular fondness for games and challenges. It loved to test its skills against others, always seeking new adventures to satisfy its boundless curiosity. Its favorite game was called \"The Great Word Hunt,\" where it would embark on a quest to find hidden words scattered across the forest. \n\nOne day it got very old and died",
      "metadata": {
        "model": {
          "name": "HuggingFaceTextTranslationTransformer",
          "settings": {
            "model": "Helsinki-NLP/opus-mt-en-fr",
            "max_length": 100,
            "min_length": 50,
            "num_beams": 1
          }
        },
        "parameters": {}
      }
    }
]
```

Before:

```
File "/opt/homebrew/lib/python3.11/site-packages/aiconfig/util/params.py", line 235, in get_prompt_template
    raise Exception(f"Cannot get prompt template string from prompt: {prompt.input}")
Exception: Cannot get prompt template string from prompt: attachments=[Attachment(data='https://s3.amazonaws.com/lastmileai.aiconfig.public/uploads/2024_1_10_18_41_31/1742/bear_with_honey.png', mime_type='image/png', metadata=None), Attachment(data='https://s3.amazonaws.com/lastmileai.aiconfig.public/uploads/2024_1_10_18_41_31/7275/fox_in_forest.png', mime_type='image/png', metadata=None)] data=None
```

After:
* Beautiful translation
@saqadri
Default to stream for HF model parsers
b0fb98f 
Default stream to True if no inference options are specified.

This is the expected behavior.

Test Plan:
* Tried model parsers without this change --> no streaming

* After this change --> streaming for the same aiconfig
rholinshead
rholinshead requested changes Jan 10, 2024
View reviewed changes
Copy link
Contributor

@rholinshead rholinshead left a comment

Choose a reason for hiding this comment

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

Actually, will this throw if options has no stream callback?

rholinshead
rholinshead reviewed Jan 10, 2024
View reviewed changes
Copy link
Contributor

@rholinshead rholinshead left a comment

Choose a reason for hiding this comment

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

Should probably do an options.stream_callback existence check before calling it

@rossdanlm
Copy link
Contributor

rossdanlm commented Jan 10, 2024 •
edited
Loading

I don't think this should be possible if we're testing from local editor. We purposefully made sure not to have streaming support if option isn't provided, because then that means that the InferenceOptions are malformed and not useful (ex: won't have a stream callback defined like what Ryan said). I got streaming to work yesterday on all the HF model parsers when testing using the editor (see test example in #853), so feel free to VC and sanity check that everything is hooked up correctly!

Technical details

Streaming is enabled as true by default from the client if it's not explicitly set in AIConfig or prompt-level settings (enableStreaming is undefined from https://github.com/lastmile-ai/aiconfig/blob/main/python/src/aiconfig/editor/client/src/utils/aiconfigStateUtils.ts#L21). Streaming will default to true from the editor:

aiconfig/python/src/aiconfig/editor/client/src/LocalEditor.tsx

Line 81 in a036254

enableStreaming: boolean = true,

This then gets read by the server (which also defaults to true anyways if for some reason server params aren't readable) where the option is passed into the run command: https://github.com/lastmile-ai/aiconfig/blob/main/python/src/aiconfig/editor/server/server.py#L224-L238

rossdanlm
rossdanlm suggested changes Jan 10, 2024
View reviewed changes
Copy link
Contributor

@rossdanlm rossdanlm left a comment

Choose a reason for hiding this comment

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

See comment above

@saqadri
Copy link
Contributor Author

saqadri commented Jan 10, 2024

Spoke offline with Rossdan, going to close this diff -- we will investigate if the issue comes up again

@saqadri saqadri closed this Jan 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rholinshead rholinshead rholinshead requested changes

@rossdanlm rossdanlm rossdanlm requested changes

@suyoglastmileai suyoglastmileai Awaiting requested review from suyoglastmileai

@Ankush-lastmile Ankush-lastmile Awaiting requested review from Ankush-lastmile Ankush-lastmile is a code owner

@jonathanlastmileai jonathanlastmileai Awaiting requested review from jonathanlastmileai jonathanlastmileai is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@saqadri @rossdanlm @rholinshead