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

Florence finetunes in workflows #794

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

Conversation

probicheaux
Copy link
Collaborator

@probicheaux probicheaux commented Nov 12, 2024

Description

A few notable changes to florence models:

  • Adds in a new (fake) "task_type" unstructured, to allow the user to type whatever they want as a prompt
  • Creates a v2 block that remaps model_version to model_id so that the user can search their finetuned florence 2 models in app
  • tweaks list of required model files to avoid always redownloading florence2 models
  • adds no_repeat_ngram_size=0 to our transformers generation args

Why we set no_repeat_ngram_size=0:

I found a very sneaky bug destroying the ability of the model to generate valid json, where this parameter was set to 3. Having the parameter set to 3 means that once the model has seen a sequence of 3 tokens, it can never repeat that sequence.

Here's the generation for a receipt json before and after making this change:
Generation before:

{'<RECEIPT>': '{"subtotal": "$27.36", "total":{"29.96"}'}

Generation after:

{'<RECEIPT>': '{"subtotal": "$27.36", "total": "$29.96"}'}

Here's why -- here's the tokens the model generated after I fixed the parameter.

['</s>', '<s>', '{"', 'sub', 'total', '":', ' "$', '27', '.', '36', '",', ' "', 'total', '":', ' "$', '29', '.', '96', '"}', '</s>']

Notice the repeated sequence of tokens:

'total', '":', ' "$',

that the model was unable to generate before this change.

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

How has this change been tested, please provide a testcase or example of how you tested the change?

tested and deployed thouroughly on localhost

Any specific deployment considerations

no

Docs

  • Docs updated? What were the changes:

@probicheaux probicheaux changed the title Florence fts working Florence finetunes in workflows Nov 12, 2024
inference/core/workflows/core_steps/common/vlms.py Outdated Show resolved Hide resolved
inference/core/workflows/core_steps/common/vlms.py Outdated Show resolved Hide resolved

class V2BlockManifest(BaseManifest):
type: Literal["roboflow_core/florence_2@v2"]
model_id: Union[WorkflowParameterSelector(kind=[ROBOFLOW_MODEL_ID_KIND]), str] = (
Copy link
Collaborator

Choose a reason for hiding this comment

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

in UI we do not display multiple versions of the same block treating newer versions as the ones shipping super-set of features - in this case (if I understand correctly) - you either choose pre-trained model (v1) or fine-tuned one (v2) - which needs to change

Copy link
Collaborator Author

@probicheaux probicheaux Nov 13, 2024

Choose a reason for hiding this comment

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

v2 can use the pre-trained ones by passing florence-2-base or florence-2-large as the model_id (same as before)! This is analogous to yolov8n-640 for object detection. This should show up in the model selector pop up but isn't -- we tried to do this in the frontend similar to yolov8n-640 or whatever, but its not working right now. Will look into it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Also the field is pre populated with the right pretrained value so ppl should not see a difference unless they try

@probicheaux
Copy link
Collaborator Author

@PawelPeczek-Roboflow good to go?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants