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

Revert "[HF][fix] Use ParameterizedModelParser instead of ModelParser" #881

Merged
merged 2 commits into from
Jan 11, 2024

Conversation

rossdanlm
Copy link
Contributor

@rossdanlm rossdanlm commented Jan 11, 2024

Lol literally a 1-line fix.

I tested by removing the model name explicitly from HF model parsers to use the default, but this errored since we're setting it to "" instead of None.
<img width="610" alt="Screenshot 2024-01-11 at 03 02 00" src="https://github.com/lastmile-ai/aiconfig/assets/151060367/1a045dbb-ad1c-46db-ba1c-32b05d923484">


## Test Plan

Before
```
model_name=""
```
I got an error saying that no model "" exists, forgot to take screenshot but it was real pls believe me


After
```
model_name=None
No model was supplied, defaulted to sshleifer/distilbart-cnn-12-6 and revision a4f8f3e (https://huggingface.co/sshleifer/distilbart-cnn-12-6).
Using a pipeline without specifying a model name and revision in production is not recommended.
```

https://github.com/lastmile-ai/aiconfig/assets/151060367/db6b35d4-9e03-4ff5-b035-c77263978fd4
Reverts #877

This caused a bug because image and audio inputs can't be parametrized. See #882 for a better fix instead
rossdanlm added a commit that referenced this pull request Jan 11, 2024
[ez][fix][HF]Explicitly set hf_model to None if it's empty string

Lol literally a 1-line fix.

I tested by removing the model name explicitly from HF model parsers to
use the default, but this errored since we're setting it to "" instead
of None.
<img width="610" alt="Screenshot 2024-01-11 at 03 02 00"
src="https://github.com/lastmile-ai/aiconfig/assets/151060367/1a045dbb-ad1c-46db-ba1c-32b05d923484">


## Test Plan

Before
```
model_name=""
```
I got an error saying that no model "" exists, forgot to take screenshot
but it was real pls believe me


After
```
model_name=None
No model was supplied, defaulted to sshleifer/distilbart-cnn-12-6 and revision a4f8f3e (https://huggingface.co/sshleifer/distilbart-cnn-12-6).
Using a pipeline without specifying a model name and revision in production is not recommended.
```


https://github.com/lastmile-ai/aiconfig/assets/151060367/db6b35d4-9e03-4ff5-b035-c77263978fd4

---
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with
[ReviewStack](https://reviewstack.dev/lastmile-ai/aiconfig/pull/879).
* #885
* #883
* #882
* #881
* __->__ #879
@saqadri
Copy link
Contributor

saqadri commented Jan 11, 2024

Yes, important to be reverted

@saqadri saqadri merged commit 3a02cb3 into main Jan 11, 2024
saqadri added a commit that referenced this pull request Jan 11, 2024
[HF][fix] Allow `kwargs` in `ModelParser.run()`



See #877 for context, and
#880 for why that original
fix needed to be reverted

Just a quick hack to get unblocked. I tried originally to make the ASR
and Image2Text ParameterizedModel but that caused other errors.

## Test Plan

Before


https://github.com/lastmile-ai/aiconfig/assets/151060367/a23a5d5c-d9a2-415b-8a6e-9826da56e985

After


https://github.com/lastmile-ai/aiconfig/assets/151060367/f29580e9-5cf6-43c5-b848-bb525eb368f2

---
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with
[ReviewStack](https://reviewstack.dev/lastmile-ai/aiconfig/pull/882).
* #885
* #883
* __->__ #882
* #881
* #879
saqadri added a commit that referenced this pull request Jan 11, 2024
…ms, get_output_text in right format (#883)

[HF][fix] Small nit fixes to ASR: Remove "model" from completion params,
get_output_text in right format


Getting an error saying that "model" param is defined twice, which it is
because we're passing it through completion params and again through the
explicit model param. This along with the change in
#879 will ensure that the
right model of `None` is passed if the value is empty string.

I also fixed typo in the aiconfig.json file


## Test Plan

I also had to change the data field from `"./hi.mp3"` to
`"cookbooks/Gradio/hi.mp3"` becuase I can only run the config from the
high level `aiconfig` directory due to this line when running the
editor:
https://github.com/lastmile-ai/aiconfig/blob/b632094618c8f96b186b1849d0036e1313e02c5a/python/src/aiconfig/scripts/aiconfig_cli.py#L126

After


https://github.com/lastmile-ai/aiconfig/assets/151060367/7387706f-deea-414b-b429-732c78322615

---
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with
[ReviewStack](https://reviewstack.dev/lastmile-ai/aiconfig/pull/883).
* #885
* __->__ #883
* #882
* #881
* #879
saqadri added a commit that referenced this pull request Jan 11, 2024
…Text2Speech (#885)

Use facebook/mms-tts-eng instead of suno/bark in Gradio cookbook for
Text2Speech

Bark is ~5GB and took my laptop 30mins to download for the first time.
The Facebook MMS
(https://huggingface.co/facebook/mms-tts-eng?fbclid=IwAR22U5VSaZ_V7QW4TJe77CM7f4fsTHjqCMjgLKV31wUl66ww1vXYSrEl-MA)
is only ~100MB which is way better and at least for cookbook I feel much
more worth it to quickly test things out. Yes the output quality is way
more memey and "voice AI" stereotype, but I feel the tradeoff is worth
it

---
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with
[ReviewStack](https://reviewstack.dev/lastmile-ai/aiconfig/pull/885).
* __->__ #885
* #883
* #882
* #881
* #879
@rossdanlm rossdanlm deleted the pr881 branch January 11, 2024 15:40
rossdanlm pushed a commit that referenced this pull request Jan 11, 2024
This is an extension of #881

Now once this is removed, only the `run-with_dependencies` kwarg should be used and this will unblock Sudhanshu from completing #664
rossdanlm added a commit that referenced this pull request Jan 11, 2024
Delete unused callback_manager argument from run commands

This is an extension of #881

Now once this is removed, only the `run-with_dependencies` kwarg should
be used and this will unblock Sudhanshu from completing
#664

---
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with
[ReviewStack](https://reviewstack.dev/lastmile-ai/aiconfig/pull/886).
* #887
* __->__ #886
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.

None yet

2 participants