[RFC][AIC-py] hf summarization model parser#740
Conversation
jonathanlastmileai
requested review from
Ankush-lastmile,
rholinshead,
rossdanlm,
saqadri and
suyoglastmileai
as code owners
| from .local_inference.text_summarization import HuggingFaceTextSummarizationTransformer | ||
|
|
||
| LOCAL_INFERENCE_CLASSES = ["HuggingFaceText2ImageDiffusor", "HuggingFaceTextGenerationTransformer"] | ||
| # from .remote_inference_client.text_generation import HuggingFaceTextGenerationClient |
There was a problem hiding this comment.
We should keep this line. It's used in the remote_inference part of this extension
There was a problem hiding this comment.
This errors for me FYI
There was a problem hiding this comment.
Huh weird, ok can you pls file issue for this and assign to me to followup?
There was a problem hiding this comment.
Can you show me this tomorrow before landing? I'm surprised
| def refine_chat_completion_params(model_settings: Dict[str, Any]) -> Dict[str, Any]: | ||
| """ | ||
| Refines the completion params for the HF text summarization api. Removes any unsupported params. | ||
| The supported keys were found by looking at the HF text summarization api. `huggingface_hub.InferenceClient.text_summarization()` |
There was a problem hiding this comment.
Nit: I know this is copied from text_generation, but can we link to the API instead?
Feel free to edit the text_generation.py file too!
| output = ExecuteResult( | ||
| **{ | ||
| "output_type": "execute_result", | ||
| "data": result["summary_text"], |
There was a problem hiding this comment.
Yea I totally see what you mean that this could definitely be centralized since so much code besides this + pipeline is shared with text_generation.py but oh well it's fine, this gets us unblocked for now!
There was a problem hiding this comment.
it's a TON of duplicated and confusing code...there's no way I could have done this reasonably quickly if it wasn't almost identical to generation
There was a problem hiding this comment.
Ok let me take a step back and show you tomorrow how model parsers are used in the main AIConfig.run(), serialize() and deserialize() functions. I'll you why model parsers are needed and how they integrate with the system
| else: | ||
| print("not str") | ||
| print(f"new_text: {new_text}") |
There was a problem hiding this comment.
Do you want to follow up with this in antoher diff? I had that if-statement as a safety check becuase sometimes it's not text outputs (not necessarily for this model parser, just generally for streaming it's not always text format) and didn't think I had time to verify. If we don't want to follow up, can you please add this comment to the issue in #742 so I don't lose track of it? Thanks
There was a problem hiding this comment.
thanks, I'll remove these
...HuggingFace/python/src/aiconfig_extension_hugging_face/local_inference/text_summarization.py
Show resolved
Hide resolved
| def _summarize(): | ||
| return summarizer(inputs, **completion_data) |
| output_data = output.data | ||
| if isinstance(output_data, str): | ||
| return output_data | ||
| if isinstance(output_data, OutputDataWithValue): |
There was a problem hiding this comment.
btw this is actually a bug that I fixed in #739. Can you change this to be OutputDataWithStringValue instead? Thanks!
I should also do a more thorough sweep of this bug, but will wait until after you land this to avoid merge conflicts
There was a problem hiding this comment.
I think it's better to fix afterward, but ideally we wouldn't duplicate so much to begin with for this kind of reason
There was a problem hiding this comment.
why does it need to be fixed afterwards? it looks like a one line change here
rossdanlm
left a comment
rossdanlm
left a comment
There was a problem hiding this comment.
pls address comments before landing, otherwise looks great, next could you also do TextTranslation task? Thanks!
Ankush-lastmile
left a comment
Ankush-lastmile
left a comment
There was a problem hiding this comment.
requesting changes to make sure that the cookbook changes do not get merged
jonathanlastmileai
force-pushed
the
pr740
branch
from
50c9408 to
ffbbbf3
Compare
|
@rossdanlm which ones are just nits ? Are we good to go? |
Very similar to text gen
Test:
```
...
{
"name": "test_hf_sum",
"input": "HMS Duncan was a D-class destroyer ...", # [contents of https://en.wikipedia.org/wiki/HMS_Duncan_(D99)]
"metadata": {
"model": {
"name": "stevhliu/my_awesome_billsum_model",
"settings": {
"min_length": 100,
"max_length": 200,
"num_beams": 1
}
}
}
},
...
}
```
```
import asyncio
from aiconfig_extension_hugging_face.local_inference.text_summarization import HuggingFaceTextSummarizationTransformer
from aiconfig import AIConfigRuntime, InferenceOptions, CallbackManager
# Load the aiconfig.
mp = HuggingFaceTextSummarizationTransformer()
AIConfigRuntime.register_model_parser(mp, "stevhliu/my_awesome_billsum_model")
config = AIConfigRuntime.load("/Users/jonathan/Projects/aiconfig/cookbooks/Getting-Started/travel.aiconfig.json")
config.callback_manager = CallbackManager([])
def print_stream(data, _accumulated_data, _index):
print(data, end="", flush=True)
async def run():
print("Stream")
options = InferenceOptions(stream=True, stream_callback=print_stream)
out = await config.run("test_hf_sum", options=options)
print("Output:\n", out)
print("no stream")
options = InferenceOptions(stream=False)
out = await config.run("test_hf_sum", options=options)
print("Output:\n", out)
asyncio.run(run())
# OUT:
Stream
Token indices sequence length is longer than the specified maximum sequence length for this model (2778 > 512). Running this sequence through the model will result in indexing errors
/opt/homebrew/Caskroom/miniconda/base/envs/aiconfig/lib/python3.10/site-packages/transformers/generation/configuration_utils.py:430: UserWarning: `num_beams` is set to 1. However, `early_stopping` is set to `True` -- this flag is only used in beam-based generation modes. You should set `num_beams>1` or unset `early_stopping`.
warnings.warn(
/opt/homebrew/Caskroom/miniconda/base/envs/aiconfig/lib/python3.10/site-packages/transformers/generation/configuration_utils.py:449: UserWarning: `num_beams` is set to 1. However, `length_penalty` is set to `2.0` -- this flag is only used in beam-based generation modes. You should set `num_beams>1` or unset `length_penalty`.
warnings.warn(
<pad> escorted the 13th Destroyer Flotilla in the Mediterranean and escorited the carrier Argus to Malta during the war The ship was recalled home to be converted into an escoter destroyer in late 1942. The vessel was repaired and given a refit at Gibraltar on 16 November, and was sold for scrap later that year. The crew of the ship escores the ship to the Middle East, and the ship is a 'disaster' of the </s>Output:
[ExecuteResult(output_type='execute_result', execution_count=0, data="<pad> escorted the 13th Destroyer Flotilla in the Mediterranean and escorited the carrier Argus to Malta during the war The ship was recalled home to be converted into an escoter destroyer in late 1942. The vessel was repaired and given a refit at Gibraltar on 16 November, and was sold for scrap later that year. The crew of the ship escores the ship to the Middle East, and the ship is a 'disaster' of the </s>", mime_type=None, metadata={})]
no stream
Output:
[ExecuteResult(output_type='execute_result', execution_count=0, data="escorted the 13th Destroyer Flotilla in the Mediterranean and escorited the carrier Argus to Malta during the war . The ship was recalled home to be converted into an escoter destroyer in late 1942. The vessel was repaired and given a refit at Gibraltar on 16 November, and was sold for scrap later that year. The crew of the ship escores the ship to the Middle East, and the ship is a 'disaster' of the .", mime_type=None, metadata={})]
```
jonathanlastmileai
force-pushed
the
pr740
branch
from
ffbbbf3 to
13e590f
Compare
I think you have a couple of comments left unresolved, I removed my request for changes. Thanks for removing changes to the cookbook. |
[AIC-py] hf mt model parser
text translation sans streaming. Streaming is not trivial here
Test:
```
import asyncio
from aiconfig_extension_hugging_face.local_inference.text_translation import HuggingFaceTextTranslationTransformer
from aiconfig import AIConfigRuntime, InferenceOptions, CallbackManager
# Load the aiconfig.
mp = HuggingFaceTextTranslationTransformer()
AIConfigRuntime.register_model_parser(mp, "translation_en_to_fr")
config = AIConfigRuntime.load("/Users/jonathan/Projects/aiconfig/test_hf_transl.aiconfig.json")
config.callback_manager = CallbackManager([])
def print_stream(data, _accumulated_data, _index):
print(data, end="", flush=True)
async def run():
# print("Stream")
# options = InferenceOptions(stream=True, stream_callback=print_stream)
# out = await config.run("test_hf_trans", options=options)
# print("Output:\n", out)
print("no stream")
options = InferenceOptions(stream=False)
out = await config.run("test_hf_trans", options=options)
print("Output:\n", out)
asyncio.run(run())
```
---
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with
[ReviewStack](https://reviewstack.dev/lastmile-ai/aiconfig/pull/753).
* __->__ #753
* #740
3 participants
[RFC][AIC-py] hf summarization model parser
Very similar to text gen
Test:
Stack created with Sapling. Best reviewed with ReviewStack.