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

πŸ“š Adding Knowledge llm blocks #50

Merged
merged 8 commits into from
Jun 27, 2024

Conversation

abhi1092
Copy link
Member

  • 🧩 Conditional Block for conditionally picking prompt templates and parsing function
  • πŸŽ›οΈ Changes in the Sampling block to handle post_fixes
  • 🐞 Bug fix for _parse function in LLMBlock

Abhi.B added 6 commits June 27, 2024 18:18
πŸ”§ Implemented conditional llmblock
πŸ› Fixed minor bugs

Signed-off-by: Abhi.B <[email protected]>
Signed-off-by: Abhi.B <[email protected]>
@abhi1092 abhi1092 changed the title πŸ“š Adding Knowledge blocks πŸ“š Adding Knowledge llm blocks Jun 27, 2024
@mergify mergify bot added the ci-failure label Jun 27, 2024
@abhi1092 abhi1092 requested a review from oindrillac June 27, 2024 19:08
Copy link
Member

@aakankshaduggal aakankshaduggal left a comment

Choose a reason for hiding this comment

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

Suggested some changes to pass the pylint

src/instructlab/sdg/llmblock.py Show resolved Hide resolved
src/instructlab/sdg/llmblock.py Show resolved Hide resolved
src/instructlab/sdg/llmblock.py Show resolved Hide resolved
src/instructlab/sdg/llmblock.py Show resolved Hide resolved
src/instructlab/sdg/llmblock.py Show resolved Hide resolved
src/instructlab/sdg/llmblock.py Outdated Show resolved Hide resolved
Signed-off-by: Aakanksha Duggal <[email protected]>
@mergify mergify bot added ci-failure and removed ci-failure labels Jun 27, 2024
@mergify mergify bot removed the ci-failure label Jun 27, 2024
Copy link
Contributor

@oindrillac oindrillac left a comment

Choose a reason for hiding this comment

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

Tested changes in this PR, works for me!

Copy link
Member

@aakankshaduggal aakankshaduggal left a comment

Choose a reason for hiding this comment

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

LGTM! πŸš€

@mergify mergify bot added the ci-failure label Jun 27, 2024
@aakankshaduggal aakankshaduggal merged commit b077b8e into instructlab:main Jun 27, 2024
10 of 11 checks passed
@russellb
Copy link
Member

I noticed that this PR changed the format expected from the dataset given to the pipeline. The CLI integration we just merged matched the old format. (We merged it after this, I just missed that this change impacted it.) This made me realize that the skills templates expect something different, as well. I filed #55 as a follow-up to this issue.

russellb added a commit to russellb/sdg that referenced this pull request Jun 30, 2024
PR instructlab#50 changed the format used in the full knowledge pipeline. Change
the simple pipelines to match.

Part of issue instructlab#55.

Signed-off-by: Russell Bryant <[email protected]>
@russellb russellb added this to the 0.1.0 milestone Jul 8, 2024
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.

4 participants