-
Notifications
You must be signed in to change notification settings - Fork 247
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
Image text pair cls #1233
Image text pair cls #1233
Conversation
…ty evaluation datasets
…nto ImageTextPairCls
}""", | ||
descriptive_stats={ | ||
"n_samples": {"test": 25010}, | ||
"avg_character_length": {"test": 1}, |
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.
Just a label?
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.
currently yes, we haven't thought much about what will be useful meta data for image datasets so it is always 1 at the moment for most datasets. This should be on the to do list
model.get_text_embeddings(texts, batch_size=encode_kwargs["batch_size"]), | ||
dim=-1, | ||
).view(num_samples, num_texts_per_sample, -1) | ||
data_loader = DataLoader( |
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.
I feel like the batching should be a model concern not a concern of the task.
We do this in main using encode_kwargs
.
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.
I would love to hear what others think though
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.
I agree
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.
The current pipeline is quite aligned with main I think? If we do results = evaluation.run(model, encode_kwargs={"batch_size": 1024}, output_folder="results"
for this task, it will be able to adjust as well (the batch size has been used in the construction of ImageTextDataset
here).
What is confusing might be that for other mieb abstask evaluators, we do something like model.get_image_embeddings(images, batch_size=encode_kwargs["batch_size"])
but for this task there is a loop over dataloader to get the images and texts out seperately first (image and text are currently both in the same Dataset Object) so it might look like the batch size is not used in the encoding process. I will aim to make it look better in the next commit.
make test
.make lint
.main
for datasets that haven't been fixed.