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

OSS work: sdk and cli support for training retriever, e2e, and qa-gen, license #33

Merged
merged 20 commits into from
Sep 20, 2023

Conversation

Ben-Epstein
Copy link
Contributor

@Ben-Epstein Ben-Epstein commented Sep 17, 2023

  • add global CLI dalm that can be called post install
  • support train e2e
  • support retriever only training
  • support qa gen on user dataset
  • update documentation everywhere for how to use dalm cli
  • fix code/mypy errors not previously handled
  • fix new linting/typing issues not caught because of argparse
  • add license

closes #22
closes #21
closes #15

README.md Outdated
```shell
git clone https://github.com/arcee-ai/DALM.git && cd DALM
python -m venv .venv && source .venv/bin/activate
pip install invoke
Copy link
Member

Choose a reason for hiding this comment

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

What is this? I have never heard of this before.

also, how can users install the dependencies?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is how you install the dependencies. I added instructions to instlal without invoke, you don't need it, but it helps organize, like a Makefile

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
dalm/training/rag_e2e/train_rage2e.py Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
LICENSE Outdated Show resolved Hide resolved
Alternatively, for development or research, you can clone and install the repo locally:
```shell
git clone https://github.com/arcee-ai/DALM.git && cd DALM
pip install --upgrade -e .
Copy link
Contributor

Choose a reason for hiding this comment

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

TIL

Copy link
Contributor

@metric-space metric-space left a comment

Choose a reason for hiding this comment

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

Other the license part and a nitpick, everything seems dandy and clean to me. Among other good things, I haven't seen this level of python type annotation/hints, awesome stuff 🥇

README.md Outdated Show resolved Hide resolved
use_peft: Annotated[bool, typer.Option(help="Whether to use Peft during fine-tuning.")] = True,
) -> None:
"""End-to-end train an in-domain model, including the retreiver and generator"""
train_e2e(
Copy link
Member

Choose a reason for hiding this comment

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

Can we make this file a bit more simplified? I feel like too many arguments passing here and there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure what you mean? It's just the 1 function. This is identical to how we have it with argparse https://github.com/arcee-ai/DALM/blob/main/dalm/training/rag_e2e/train_rage2e.py#L58-L211

Why is this confusing but argparse isn't?

README.md Outdated Show resolved Hide resolved
@arcee-ai arcee-ai deleted a comment from shamanez Sep 20, 2023
@Ben-Epstein Ben-Epstein merged commit 01d4ff8 into main Sep 20, 2023
1 check passed
@Ben-Epstein Ben-Epstein deleted the feat/oss-cli-sdk-support branch September 21, 2023 22:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Get rid of our sys.path.append in train_rage2e.py Add a dalm cli for users Readme documentation
3 participants