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

Support for local models (+ Other OpenAI compatible end-points) #32

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

tijszwinkels
Copy link

These changes allow to use other OpenAI compatible api endpoints by setting the OPENAI_BASE_URL and OPENAI_API_KEY env. variable.

See Readme for more details.

requirements.txt Outdated
@@ -4,3 +4,4 @@ loguru
datasets
typer
rich
cffi

Choose a reason for hiding this comment

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

Remove this

Copy link
Author

@tijszwinkels tijszwinkels Jul 26, 2024

Choose a reason for hiding this comment

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

In my testing, this seems required through the 'datasets' import:

Without it:

conda create --name moa-test python=3.11
conda activate moa-test
pip install -r requirements.txt
tijs@Perplexity MoA % DEBUG=1 python bot.py --model "groq/mixtral-8x7b-32768" --reference-models "groq/llama3-70b-8192" --reference-models "groq/mixtral-8x7b-32768" --reference-models "groq/gemma2-9b-it" --rounds 2  
Traceback (most recent call last):
  File "/Users/tijs/os/MoA/bot.py", line 1, in <module>
    import datasets
  File "/opt/homebrew/anaconda3/envs/moa-test/lib/python3.11/site-packages/datasets/__init__.py", line 17, in <module>
    from .arrow_dataset import Dataset
  File "/opt/homebrew/anaconda3/envs/moa-test/lib/python3.11/site-packages/datasets/arrow_dataset.py", line 75, in <module>
    from . import config
  File "/opt/homebrew/anaconda3/envs/moa-test/lib/python3.11/site-packages/datasets/config.py", line 145, in <module>
    importlib.import_module("soundfile").__libsndfile_version__
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/anaconda3/envs/moa-test/lib/python3.11/importlib/__init__.py", line 126, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/tijs/.local/lib/python3.11/site-packages/soundfile.py", line 17, in <module>
    from _soundfile import ffi as _ffi
  File "/Users/tijs/.local/lib/python3.11/site-packages/_soundfile.py", line 2, in <module>
    import _cffi_backend
ModuleNotFoundError: No module named '_cffi_backend'
(moa-test) 

Copy link
Author

Choose a reason for hiding this comment

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

Actually, you're right. This looks like a bug in whatever version of 'datasets' I happened to pick up. I removed it.

So: Done!

@@ -116,13 +116,49 @@ The CLI will prompt you to input instructions interactively:

You can configure the demo by specifying the following parameters:

- `--aggregator`: The primary model used for final response generation.

Choose a reason for hiding this comment

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

Why are we renaming this parameter?

Copy link
Author

@tijszwinkels tijszwinkels Jul 26, 2024

Choose a reason for hiding this comment

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

the documentation is incorrect. Typer will just get the parameters from the main function, where 'model' is used to define the primary model.

I changed the code to use --aggregator instead of --model

bot.py Outdated
@@ -118,7 +118,7 @@ def main(

model = Prompt.ask(
"\n1. What main model do you want to use?",
default="Qwen/Qwen2-72B-Instruct",
default=model,

Choose a reason for hiding this comment

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

Suggest reverting to "aggregator"

Copy link
Author

Choose a reason for hiding this comment

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

Done!

@jkennedyvz
Copy link

Thanks @tijszwinkels I look forward to using this

@tijszwinkels
Copy link
Author

tijszwinkels commented Jul 26, 2024

@jkennedyvz I'll leave it up to you to resolve the conversations, but I think this should be it for the review:

  • I renamed the --model option to --aggregator as requested
  • Removed the 'cffi' requirement (Seems like a problem with the datasets library? In any case, should be fixed elsewhere)

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.

2 participants