-
Notifications
You must be signed in to change notification settings - Fork 289
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
Feat: Add jasper #1591
Feat: Add jasper #1591
Conversation
I think my implementation of the model has lower results on |
I think it is perfectly fine to apply prompts only to some tasks (as long as it is clear in the implementation) |
I agree, but it unclear why passage prompt not applying to retrieval only. Should they be applied to InstructionsRetrieval or InstructionReranking? |
Yea that is a somewhat arbitrary decision (again that is why it is nice to have the implementation). I would probably add it in both cases |
I suggest waiting for @DunZhang's input to hear his opinion on this |
@Samoed In Englinsh-MTEB, the Rerank tasks really more like STS tasks, which means that the queries and 'passages' are symmetrical. Below is the task type about As the data are symmetrical, they all need prompt just like STS task! On the contrary, Rerank task in Chinese-MTEB is about query and passage (Irrelevance to the present matter, not to be discussed at length). Finally, my model's usage: For s2p task (e.g. retrieval), s need prompt, p does not need prompt For s2s task (e.g. STS), they all need prompt. Reference: |
As for the other mismatched tasks, that's too hard to explain, and if the overall difference in averages isn't too large, I think it's negligible. Below is some reproduction details:
Finally:
Actually, convert to fp32 then do normalize always get high score(Statistically non-significant difference) |
I think I will try to apply prompt based on retrieval type. Thank you for the feedback! |
# Conflicts: # mteb/models/overview.py
instruction = self.get_task_instruction(task_name, prompt_type) | ||
|
||
# to passage prompts won't be applied to passages | ||
if prompt_type == PromptType.passage and task.metadata.type == "s2p": |
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've updated it to apply the passage prompt only if the task type is s2s
or p2p
.
Just noting here, that it is perfectly valid to change the prompt conditional on the task. E.g.
|
Yes, I've changed and now prompt won't apply to all |
Then I believe this is ready to merge? |
I think yes, but I was waiting if @DunZhang have something to add |
Hi. I have nothing more to add 😄 |
Perfect - will merge then - thanks for taking the time |
Checklist
make test
.make lint
.Adding a model checklist
mteb.get_model(model_name, revision)
andmteb.get_model_meta(model_name, revision)
Full model results embeddings-benchmark/results#68
Some results are not matching, with a significant gap in
AskUbuntuDupQuestions
. @DunZhang, could you please take a look at what might be wrong?I tried adding:
but it had no effect. I also tried setting
max_seq_length
to 400, but that didn’t help either.Full results jasper_results.zip
Eval code