dspy
dspy copied to clipboard
Support convenience import for `OllamaLocal`
Summary
tldr; These changes add a convenience import for the OllamaLocal
import introduced in https://github.com/stanfordnlp/dspy/pull/262 consistent with what's done for GPT-3.5, et al. I also added a minimal test to show the before/after comparison.
I began working through intro.ipynb
today and wanted to use a model I was hosting locally with Ollama instead of GPT-3.5. Though the OpenAI
symbol is actually a reference to dsp.modules.GPT3
, there's some hoisting to make it a little more convenient to access:
turbo = dspy.OpenAI(model='gpt-3.5-turbo')
I expected to do the same for Ollama:
nous_hermes2_mixtral = dsp.OllamaLocal(model="nous-hermes2-mixtral", model_type="chat")
but found it wasn't supported. I've made the appropriate change to dsp/modules/__init__.py
to make the Ollama module consistent with its siblings.
I wanted to add a test demonstrating the change, but couldn't find any unit tests. I've added a new tests/
package at the root of the repo to host all test suites, which I understand to be common convention. I've avoided adding any dependencies by relying on the built-in unit testing library in Python.
⚠️ These changes also propose running the new unit test as part of GitHub CI checks. I'm unfamiliar with GitHub's CI system, so if the CI change is acceptable, it deserves some scrutiny to make sure I didn't make a silly mistake.
Testing
I added a unit test that verifies the convenience import is now supported. Unit tests can be executed from the command line with this command:
% python -m unittest discover tests/
Before:
E
======================================================================
ERROR: test_convenience_import_of_dsp_ollama_client (test_dsp.DspModuleImportsTest.test_convenience_import_of_dsp_ollama_client)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/Users/forwardprogress/Developer/stanfordnlp/dspy/tests/test_dsp.py", line 6, in test_convenience_import_of_dsp_ollama_client
from dsp import OllamaLocal
ImportError: cannot import name 'OllamaLocal' from 'dsp' (/Users/forwardprogress/Developer/stanfordnlp/dspy/dsp/__init__.py)
----------------------------------------------------------------------
Ran 1 test in 0.479s
FAILED (errors=1)
After:
.
----------------------------------------------------------------------
Ran 1 test in 0.484s
OK
Hey thanks so much, but this may create a new dependency for everyone who doesn't need it?
Hey thanks so much, but this may create a new dependency for everyone who doesn't need it?
Ah, that's a great point that I didn't fully consider. I think the Ollama module requires no additional dependencies but the HF change does.
If I updated the changes to only hoist OllamaLocal
, would that be agreeable?
To be clear, I'm operating with the presumption that it's desirable for most (or all?) modules to have these convenience imports. If that's not the case, I'm happy to get that feedback.
Whelp, this PR got whittled down to a lot less than I expected. 😉
Alright @okhat @piotrlaczkowski I don't think I'm getting traction with this change so I'll plan to close it out sometime tomorrow if I don't hear back.
No reason to not merge this, it's a bugfix IMO!
I’m confused. This creates a dependency on other tools that everyone now will need to install?
From ollama.py
from dsp.modules.lm import LM
from typing import Any, Literal, Optional
import os, multiprocessing, datetime, hashlib
import requests
import json
Aren't those already there/ in the standard lib? Sorry if I'm missing the obvious 😅
This creates a dependency on other tools that everyone now will need to install?
Nope!
I think there may be some confusion that this adds a dependency on Ollama's brand-new ollama
package, but this isn't that. The OllamaLocal
client I'm referring to in this PR's discussion is already implemented in dspy
without dependencies on other packages.
https://github.com/stanfordnlp/dspy/blob/967bf9670a6bf5730f8c71378604847253940e26/dsp/modules/ollama.py#L23-L25
This change just makes it as easy to access the client as easy as it is to access the OpenAI
client.
turbo = dspy.OpenAI(model='gpt-3.5-turbo')
The change makes access to LM clients a little more consistent, but it's easy to access the OllamaLocal
client by writing this instead:
from dsp.modules.ollama import OllamaLocal
That being said, this a low-value change and it's not worth distracting @okhat with such a mundane change. Maybe later on, I'll look into what it would look to adopt the new official Ollama client.
Cheers!