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 faiss-cpu version range #1097

Merged
merged 1 commit into from
Nov 11, 2024
Merged

Conversation

dlqqq
Copy link
Member

@dlqqq dlqqq commented Nov 8, 2024

Description

This PR updates the version range of faiss-cpu to:

  1. Include all versions with Python 3.12 support (Python 3.12 not supported #538)
  2. Exclude versions lacking wheels for all supported platforms (Jupyter AI installation failing on x86 macOS #858)
  3. Allow for newer minor releases of faiss-cpu~=1.0, notably faiss-cpu==1.9.0.

After this PR is merged and released in a patch version, we can update the dependency in the Conda Forge recipe similarly to close #1096.

Testing instructions

  1. Run jlpm dev-uninstall && jlpm dev-install.
  2. Run pip install -U faiss-cpu.
  3. Run pip show faiss-cpu, then verify that the latest release v1.9.0 is installed.
  4. Verify /learn and /ask still work in Jupyter AI.

@dlqqq dlqqq added bug Something isn't working dependency:faiss-cpu Issues pertaining to `faiss-cpu` labels Nov 8, 2024
@krassowski krassowski linked an issue Nov 11, 2024 that may be closed by this pull request
@krassowski
Copy link
Member

Great! But why is faiss-cpu a required dependency in the first place?

@dlqqq
Copy link
Member Author

dlqqq commented Nov 11, 2024

@krassowski Good callout. The original line of thought was that /learn was a built-in slash command, and therefore it had to work "out-of-the-box". Therefore we listed faiss-cpu as a required dependency.

I agree that we should explore making faiss-cpu an optional dependency. This would likely be considered a breaking change for admins deploying Jupyter AI as a service (since users may not have permissions to install faiss-cpu and restart the server). I'll open an issue for this and add it to the v3.0.0 roadmap.

Copy link
Collaborator

@srdas srdas left a comment

Choose a reason for hiding this comment

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

Tested all steps in the PR.

  1. Version = faiss-cpi 1.9.0
  2. /learn works
  3. /ask works

@dlqqq
Copy link
Member Author

dlqqq commented Nov 11, 2024

@srdas Verified locally as well. Thank you for verifying this PR on your machine.

@krassowski I've opened a new issue to track making faiss-cpu optional: #1100

@dlqqq dlqqq merged commit d5d604d into jupyterlab:main Nov 11, 2024
13 checks passed
@dlqqq
Copy link
Member Author

dlqqq commented Nov 11, 2024

@meeseeksdev please backport to v3-dev

meeseeksmachine pushed a commit to meeseeksmachine/jupyter-ai that referenced this pull request Nov 11, 2024
dlqqq added a commit that referenced this pull request Nov 11, 2024
Co-authored-by: david qiu <david@qiu.dev>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working dependency:faiss-cpu Issues pertaining to `faiss-cpu`
Projects
None yet
Development

Successfully merging this pull request may close these issues.

jupyter-ai2.28 still does not support python3.13.0 conda installing jupyter-ai fails with TypeError
3 participants