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

refactor model class and runners to be more independent #494

Open
wants to merge 26 commits into
base: master
Choose a base branch
from

Conversation

deigen
Copy link
Contributor

@deigen deigen commented Jan 23, 2025

Currently, the ModelRunner is the primary object of instantiation for a model in a server. This has several drawbacks:

  • The user can't directly create their model without also creating a Runner, unless always including boilerplate workarounds. Creating a model directly is useful for testing and local use.
  • It's cumbersome to create gRPC-served models, as they also require instantating a Runner class.
  • The instantiation process of the Model in the server isn't clear, at no point is there a straightforward call to model = Model(**args); the init is instead the Runner init.

Instead, we should define the model implementation using ModelClass directly, and create Runner and gRPC servers that can each call the Model object. This separates the model class from the server method, while still maintaining the same interfaces.

NOTE: This change also requires us to change the base class of all models in the examples repo, to use ModelClass instead of ModelRunner as their base class.

@deigen deigen requested review from zeiler and luv-bansal January 23, 2025 17:41
@deigen deigen force-pushed the model-class-refactor branch 3 times, most recently from ebd1ca8 to 19e18db Compare January 24, 2025 03:05
@deigen deigen force-pushed the model-class-refactor branch from 19e18db to e851890 Compare January 24, 2025 03:08
@deigen deigen requested a review from ackizilkale January 24, 2025 19:04
Copy link

Code Coverage

Package Line Rate Health
clarifai 43%
clarifai.cli 65%
clarifai.client 71%
clarifai.client.auth 74%
clarifai.constants 100%
clarifai.datasets 100%
clarifai.datasets.export 80%
clarifai.datasets.upload 75%
clarifai.datasets.upload.loaders 37%
clarifai.models 100%
clarifai.modules 0%
clarifai.rag 72%
clarifai.runners 12%
clarifai.runners.models 57%
clarifai.runners.utils 66%
clarifai.schema 100%
clarifai.urls 80%
clarifai.utils 77%
clarifai.utils.evaluation 67%
clarifai.workflows 94%
Summary 68% (4411 / 6526)

Minimum allowed line rate is 50%

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant