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

Update dependency list and move optional ones to extra dependencies #80

Merged
merged 17 commits into from
May 26, 2024

Conversation

ProKil
Copy link
Member

@ProKil ProKil commented May 24, 2024

Close #59

This is the start of an ongoing effort to make sotopia dependency slimmer.

Similar to tinygrad's point, having less dependencies will make more people easy to use our library with other tools that they are using (no need to resolve conflicts that don't actually exist).

I think the only 8 dependencies that we should have are:

  1. pydantic: for data validation and abstraction,
  2. redis-om: for data serialization, logging, and communication,
  3. tqdm: for fun,
  4. aiohttp: for calling RESTAPI,
  5. beartype: for type guarding,
  6. lxml: for handling different visibilities
  7. jinja2: for prompt templates
  8. and gin: for configuration, and we should remove it in the end as well.

Options:

[example]:

  • pandas
  • transformers
  • datasets
  • scipy
  • torch

[litellm]:

  • litellm

[chat]:

  • fastapi

[testing]:

  • mypy
  • pytest
  • pytest-asyncio

[dev]:

  • ruff
  • pre-commit

📑 Description

In this PR, I made progress towards this goal --

This is the current required dependency list

pandas = "^2.1.1"
lxml = "^4.9.3"
openai = "^1.11.0"
langchain = "0.1.5"
rich = "^13.6.0"
PettingZoo = "1.24.0"
redis-om = "^0.2.1"
pandas-stubs = ""
types-tqdm = "
"
gin-config = "^0.5.0"
absl-py = "^2.0.0"
together = "^0.2.4"
pydantic = "1.10.12"
beartype = "^0.14.0"
langchain-openai = "^0.0.5"
litellm = "^1.23.12"

We now have 14 required dependencies (except for stubs), and removing the other 6 is hard for the following reasons.

  1. pandas is only used for converting a dict into a csv. This should be a feature that we can easily implement.
  2. openai and together are our "default" LLMs which we could do without, but requires more changes.
  3. rich for printing, which we could conditionally use.
  4. PettingZoo is our initial abstraction for environments, which we have in fact deviated from. We could do without it in a small PR.
  5. litellm could be optional in a following PR

Here are our optional dependencies:

fastapi = { version = "^0.109.2", optional = true }
tabulate = { version = "^0.9.0", optional = true }
anthropic = { version = "^0.26.0", optional = true }
xmltodict = { version = "^0.13.0", optional = true }
groq = { version = "^0.4.2", optional = true }
cohere = { version = "^5.1.8", optional = true }
google-generativeai = { version = "^0.5.4", optional = true }
transformers = { version = "^4.41.0", optional = true }
datasets = { version = "^2.19.0", optional = true }
scipy = { version = "^1.13.1", optional = true }
torch = { version = "^2.3.0", optional = true }

✅ Checks

  • My pull request adheres to the code style of this project
  • My code requires changes to the documentation
  • I have updated the documentation as required
  • All the tests have passed
  • Branch name follows type/descript (e.g. feature/add-llm-agents)
  • Ready for code review

ℹ Additional Information

@ProKil ProKil requested a review from XuhuiZhou May 24, 2024 19:35
Copy link
Member

@XuhuiZhou XuhuiZhou left a comment

Choose a reason for hiding this comment

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

Do we have to remove datasets?

@ProKil ProKil changed the title Remove torch and 4 other dependencies Update dependency list and move optional ones to extra dependencies May 26, 2024
@ProKil
Copy link
Member Author

ProKil commented May 26, 2024

@XuhuiZhou

I added datasets back to examples extra. You will be able install it with either pip install sotopia[examples] or poetry install -E examples.

Now we have two different kinds of dependencies:

  1. Required dependencies are for the included files -- ./sotopia.
  2. Optional dependencies (extras) for either:
    1. external LLM support
    2. OR scripts outside of the sotopia, incl. examples and sotopia-chat

@ProKil ProKil requested a review from lwaekfjlk May 26, 2024 15:39
Copy link
Member

@XuhuiZhou XuhuiZhou left a comment

Choose a reason for hiding this comment

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

Nice, thanks!
Can you add relevant instructions for installing the optional to the doc?
Also,
Consider removing the lmlib folder?

@ProKil
Copy link
Member Author

ProKil commented May 26, 2024

lmlib is removed already? I will reply after I add the docs

Copy link
Member

@lwaekfjlk lwaekfjlk left a comment

Choose a reason for hiding this comment

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

Nice update for the whole project!

.github/workflows/tests.yml Show resolved Hide resolved
@ProKil
Copy link
Member Author

ProKil commented May 26, 2024

@XuhuiZhou the doc is added

@ProKil ProKil force-pushed the feature/remove-unused-deps branch from 98e5785 to 1ec13d1 Compare May 26, 2024 19:30
@ProKil
Copy link
Member Author

ProKil commented May 26, 2024

@XuhuiZhou @lwaekfjlk Please check the current code and approve if it looks good.

As for the efficiency of CI, I think it currently looks OK since we spend around 7 minutes per push which is around 300 pushes per month or 10 pushes per day. We can make this more efficient in a separate PR according to the various solutions in #79

@lwaekfjlk
Copy link
Member

@XuhuiZhou @lwaekfjlk Please check the current code and approve if it looks good.

As for the efficiency of CI, I think it currently looks OK since we spend around 7 minutes per push which is around 300 pushes per month or 10 pushes per day. We can make this more efficient in a separate PR according to the various solutions in #79

yes, sure. Sorry, I just checked again. 2000 min CI is for all private repo, public ones are not included.

@ProKil ProKil added this to the 0.1.0 Release milestone May 26, 2024
Copy link
Member

@lwaekfjlk lwaekfjlk left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@XuhuiZhou XuhuiZhou merged commit 184d1ca into main May 26, 2024
10 checks passed
@ProKil ProKil deleted the feature/remove-unused-deps branch May 28, 2024 20:42
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.

[FEAT]: Get rid of torch requirement for sotopia
3 participants