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

feat: Caraml sdk commons #8

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

Conversation

maniteja6799
Copy link
Collaborator

@maniteja6799 maniteja6799 commented Feb 21, 2024

  • CaraML client created that wraps on top of existing models, turing clients
  • MLP module created, which will be used by all the other clients (caraml, models, turing)
  • modules moved to a new package, making imports look like from caraml.models.transformer import Transformer
  • Sample notebook added to showcase caraml client usage caraml-sdk/examples/sklearn

[Existing Issues]

  • Routers client is commented in caraml client as of now because of the following issue:

All over turing wrapper code, the imports are not used well. There are pieces like this response = caraml.routers.active_session.list_ensembling_jobs( ...) all through the code. I've fixed all the instances for in-code imports, But there are these usages for active sessions using global static objects inside a class definition. The package is inconsistent with different formats used at different parts of the turing code. All the classes are using a global active_session object added in the turing package init.py file

PS: Please ignore the number of commits in this PR. I've created a branch from my forked repo, and the merge in the base repo is a squashed commit.

maniteja6799 and others added 30 commits February 5, 2024 22:10
chore: sync routers client sdk from Ref:main
… later to the client metadata to track app version
maniteja6799 and others added 21 commits February 28, 2024 11:40
chore: sync models client sdk from caraml-dev/merlin:v0.39.1
chore: sync routers client sdk from caraml-dev/turing:v1.17.1
chore: sync mlp client sdk from caraml-dev/mlp:v1.12.0
@maniteja6799 maniteja6799 self-assigned this Mar 5, 2024
@maniteja6799 maniteja6799 requested review from mbruner and leonlnj March 5, 2024 05:23
@maniteja6799 maniteja6799 marked this pull request as ready for review March 5, 2024 05:23
@maniteja6799
Copy link
Collaborator Author

Note: Pytests in models, routers are failing.

For Models package. there are issues with pytest only since it looks for current module first packages while figuring imports and there is a conflict in names for the current caraml-sdk packages (caraml.routers/models/common.*) vs caraml upi packages loaded into site-packages under sys path (caraml.upi.*).

Routers tests are failing due to various reasons, After the refactor to use the caraml package as the base package for import statements inside wrapper code, there are discrepancies and inconsistencies in some files that are making issues for tests to run.

@maniteja6799
Copy link
Collaborator Author

For Models package. there are issues with pytest only since it looks for current module first packages while figuring imports and there is a conflict in names for the current caraml-sdk packages (caraml.routers/models/common.) vs caraml upi packages loaded into site-packages under sys path (caraml.upi.).

Regarding this, trying out some monkey patching to get caraml.upi working like mentioned here: pytest-dev/pytest#5147 (comment)

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.

1 participant