-
Notifications
You must be signed in to change notification settings - Fork 26.3k
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
Adding new zero-shot examples #32483
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: amyeroberts <[email protected]>
Co-authored-by: amyeroberts <[email protected]>
Co-authored-by: amyeroberts <[email protected]>
Co-authored-by: amyeroberts <[email protected]>
Co-authored-by: amyeroberts <[email protected]>
Co-authored-by: amyeroberts <[email protected]>
Co-authored-by: amyeroberts <[email protected]>
Co-authored-by: amyeroberts <[email protected]>
Co-authored-by: amyeroberts <[email protected]>
Co-authored-by: amyeroberts <[email protected]>
@qubvel Hi Pavel this PR is ready for review. However, there is one problem with using evaluation process in In order to use the transformers/src/transformers/trainer.py Line 3865 in aefd3e2
There is three ways for this
|
Hi @SangbumChoi, thanks for working on this!
I think it is fine to have batch size 2 if the model can be trained with such a batch size. In README and a comment we can mention how much memory one would require to run the script and also advise using gradient accumulation, lower precision, and multi-GPU setup (if it is supported). |
We can probably move these lines main_input_name = getattr(self.model, "main_input_name", "input_ids")
inputs_decode = self._prepare_input(inputs[main_input_name]) if args.include_inputs_for_metrics else None to a separate method of |
agreed re; |
Let me work on pavel's suggestion and ping again. Thanks for the direction! |
…to grounding_examples
@qubvel Hi requesting for first review. I will state changes from the original object_detection.py
Also kudos to @EduardoPach for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @SangbumChoi, thanks for iterating! Great work!
I left a comment regarding Trainer, also don't know if it deserves a separate PR. Overall looks very good to me, but I didn't do an in-depth view yet (let's wait for loss PR to be merged). For these examples the tests have to be added and information in README (see files in this directory). Also, feel free to add resources to docs of Grounding DINO. Btw, did you try to fine-tune OwlV2?
@qubvel Sounds good. I can wait until the loss PR get merged.
I didn't aware that we can fine-tune OwlV2, let me see while waiting loss PR! |
post_processed_output = processor.post_process_grounded_object_detection( | ||
output, input_ids, box_threshold=box_threshold, text_threshold=text_threshold, target_sizes=target_sizes | ||
) | ||
post_processed_output = convert_zero_shot_to_coco_format(post_processed_output, label2id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@SangbumChoi Not sure if this will still work if random_text_promt=True
because the class_labels
from targets will not necessarily follow label2id
…ach/transformers into grounding_examples
What does this PR do?
Fixes #32459
Following two PR should be merged before merging this example!
#31828
#31964
Before submitting
Pull Request section?
to it if that's the case.
documentation guidelines, and
here are tips on formatting docstrings.
Who can review?
Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.