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

Improvements for maintainability #24

Open
4 of 5 tasks
amrit110 opened this issue Jan 29, 2025 · 12 comments
Open
4 of 5 tasks

Improvements for maintainability #24

amrit110 opened this issue Jan 29, 2025 · 12 comments
Assignees

Comments

@amrit110
Copy link
Member

amrit110 commented Jan 29, 2025

vector-inference is starting to expand to more models and has onboarded users. I'm looking to contribute by helping improve its maintainability. Concretely, I have 4 tasks in mind:

  • Reduce maintenance overhead by having dependabot submit security updates and dependency updates as long as they conform to the requirements of the library
  • Add a docs build to create a webpage to help users
  • Improve the dev build tooling (migrate to uv from poetry)
  • Add unit tests for the python code in the vec-inf package.
  • Automate docker image build process and pushing to dockerhub/GCR repo
@amrit110 amrit110 self-assigned this Jan 29, 2025
@amrit110
Copy link
Member Author

#23 for dependabot configuration

@amrit110
Copy link
Member Author

#28 for migrating to uv

@amrit110
Copy link
Member Author

amrit110 commented Feb 3, 2025

#30 for docs build

@jwilles
Copy link

jwilles commented Feb 3, 2025

One issue that I will add for discussion - VI is slow to onboard new models because of the strong coupling between code and configuration. We currently bundle the job config with the VI package. This means that users need update their VI package to add a few lines to their models.csv which doesn't feel great.

@jwilles
Copy link

jwilles commented Feb 18, 2025

@XkunW We are also missing developer documentation. The user facing documentation is relatively clear but it would be good to expand the documentation with a contribution guide.

@amrit110
Copy link
Member Author

@jwilles, I have submitted a PR #42 that addresses the issue you raised. I have added you to review as well. I haven't actually tested a custom model yet on the cluster. I will do that shortly to make sure it works.

@amrit110
Copy link
Member Author

Update @jwilles, @XkunW I tested a custom model - https://huggingface.co/Qwen/Qwen2.5-7B-Instruct-1M. Works! I will be updating the documentation shortly on how users can onboard their own models.

@rohan-uiuc
Copy link

I like this package and quite interested to make it work for my use case, just want to confirm if there are any plans to support the following use cases?

  1. Programmatic API access instead of json mode in cli
  2. Cluster specific information fetched from a config file, for eg. .sif file location, module loads for cuda/cudnn, partition used for models, gpus per model as this can differ based on partition/gpu type
  3. Support for multiple clusters, maybe it can be made possible using the same config I mentioned in point 2

Thanks for your work, it's quite useful :)

@amrit110
Copy link
Member Author

amrit110 commented Mar 2, 2025

@rohan-uiuc thanks for your interest. Can you elaborate on 1. and give an example usage? If I understand it better, perhaps it can be a feature request for us. Regarding 2. and 3. I think my colleague @jwilles might have a better idea.

@rohan-uiuc
Copy link

@amrit110 thanks for the quick response. About 1, it's about the ability to enable using this package natively via python scripts/applications. I was playing around yesterday, let me open a PR for your reference. I didn't have a great way to test as it will need several manual changes to make it work on my cluster, which I might attempt in the coming week.

  1. will be most useful for any devs like me who have access to different clusters and can get the package up and running with minimal changes to a single config file. Would love some help there.

@rohan-uiuc
Copy link

I've opened #54 . Please feel free to leave comments or make improvements as I cannot manually test it yet. I will be happy to contribute with some more features like adding an apptainer .def and automatically building it if .sif is not found. But I would need point 2. as a prerequisite to able to test my contributions.

@XkunW
Copy link
Contributor

XkunW commented Mar 3, 2025

I've opened #54 . Please feel free to leave comments or make improvements as I cannot manually test it yet. I will be happy to contribute with some more features like adding an apptainer .def and automatically building it if .sif is not found. But I would need point 2. as a prerequisite to able to test my contributions.

@rohan-uiuc Thanks for your contribution! Regarding point 2 and 3, we have plans on the roadmap to decouple the config with our current cluster setup for better generalizability, so stay tuned on that. We're also in the process of automating the deployment of the singularity container (.sif file). The singularity container is only one way of using vector inference, python venvs are also accepted.

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

No branches or pull requests

4 participants