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

[Good First Issue]: Verify dolly-v2-3b with GenAI text_generation #275

Open
p-wysocki opened this issue Mar 1, 2024 · 18 comments · Fixed by #294
Open

[Good First Issue]: Verify dolly-v2-3b with GenAI text_generation #275

p-wysocki opened this issue Mar 1, 2024 · 18 comments · Fixed by #294
Labels
good first issue Good for newcomers

Comments

@p-wysocki
Copy link
Collaborator

Context

This task regards enabling tests for dolly-v2-3b. You can find more details under openvino_notebooks LLM question answering README.md.

Please ask general questions in the main issue at #259

What needs to be done?

Described in the main Discussion issue at: #259

Example Pull Requests

Described in the main Discussion issue at: #259

Resources

Contact points

Described in the main Discussion issue at: #259

Ticket

No response

@Vishwa44
Copy link
Contributor

Vishwa44 commented Mar 3, 2024

.take

@Vishwa44
Copy link
Contributor

Vishwa44 commented Mar 6, 2024

Hi @Wovchena can this task be assigned to me

@p-wysocki
Copy link
Collaborator Author

Hello @Vishwa44, we enabled the take command in GenAI repo just after you wrote it. :)

I assigned you the task, thanks for taking a look! Please let us know if you have any questions.

@Vishwa44
Copy link
Contributor

Vishwa44 commented Mar 7, 2024

@p-wysocki I just got done testing the model. It's working as expected
I'll raise a pr adding it to the supported model list

@p-wysocki
Copy link
Collaborator Author

Perfect! Please make sure to link it to the issue. :)

@Vishwa44
Copy link
Contributor

Vishwa44 commented Mar 8, 2024

Hi @p-wysocki,
this is the link to my pr verifed-dolly

@p-wysocki
Copy link
Collaborator Author

Hello @Vishwa44, thank you for your contribution! Could you please add tests? You can check other completed issues for reference or the issue description at #259.

@Vishwa44
Copy link
Contributor

Hi @p-wysocki
I'm really sorry but I'm new to this, can you share some handy guide on how to add tests for github actions runner, I'll make the required changes and get it done asap

@mlukasze
Copy link

mlukasze commented Mar 19, 2024

@Wovchena || @pavel-esir : could you please close a ticket, both PRs has been merged.
Also, please add 2024.1 as a milestone to this ticket.
Thanks.

@p-wysocki p-wysocki linked a pull request Mar 19, 2024 that will close this issue
@pavel-esir
Copy link
Contributor

pavel-esir commented Mar 19, 2024

@mlukasze those prs were for Readme changes, but there is a left test for dolly 2-3b. Let's close after the tests are implemented.
@Vishwa44 sorry for late reply, you can take as example already existing test where we compare output generated by us with HF transformers output, e.g.:
https://github.com/openvinotoolkit/openvino.genai/blob/master/.github/workflows/causal_lm_cpp.yml#L151-L168

you can simplify code above and compare only for greedy scenario, run greedy_causal_lm instead of beam_search_causal_lm and generate in HF with much shorter command generate(**tokenized, max_length=100, do_sample=False)

Please let me know if you have any questions

@mlukasze
Copy link

got it, thanks for clarification :)

@Vishwa44
Copy link
Contributor

@pavel-esir I'll raise the pr asap for adding the test, thank you

@andrei-kochin
Copy link
Collaborator

@Vishwa44 are you still working on this? there is more than month without an update

@p-wysocki p-wysocki assigned Vishwa44 and unassigned Vishwa44 May 6, 2024
@p-wysocki
Copy link
Collaborator Author

The model still needs to be validated and the task is now open again. @Vishwa44 if you're still working on this please let us know, for now I reopened the task.

@Vishwa44
Copy link
Contributor

Vishwa44 commented May 6, 2024

@p-wysocki sorry for the late reply, I'll resume on this and finish this task

@Apoorv012
Copy link

@p-wysocki Is this issue up for grab?

@Apoorv012
Copy link

.take

Copy link

Thank you for looking into this issue! Please let us know if you have any questions or require any help.

@Apoorv012 Apoorv012 removed their assignment Jun 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
Status: Assigned
Development

Successfully merging a pull request may close this issue.

6 participants