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.

Sign up for GitHub

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

Jump to bottom

faiss: 1.8.0->1.9.0 #341418

Merged
merged 1 commit into from
Oct 9, 2024
Merged

faiss: 1.8.0->1.9.0 #341418

merged 1 commit into from
Oct 9, 2024
+8 −52

Conversation

sarahec
Copy link
Contributor

@sarahec sarahec commented Sep 12, 2024 •
edited
Loading

Description of changes

  • Upgraded version 1.8.0 with 1.9.0.
  • Removed passthru testing. This simply ran one of the demo programs. That porogram is no longer maintained and doesn't compile. The newer demos require manual setup and thus aren't suitable.

Downstream effects

  1. Unblocks building on Darwin
  2. Supports SWIG.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@sarahec sarahec marked this pull request as draft September 12, 2024 17:00
@sarahec sarahec added 6.topic: python 6.topic: darwin Running or building packages on Darwin labels Sep 12, 2024
@ofborg ofborg bot requested a review from SomeoneSerge September 12, 2024 22:22
@sarahec
Copy link
Contributor Author

sarahec commented Sep 13, 2024

Awaiting the outcome of facebookresearch/faiss#3846 before opening for review.

@emilazy
Copy link
Member

emilazy commented Sep 13, 2024

Is this a SWIG 4 thing? We may be able to apply whatever upstream patch fixed it to the stable release.

@sarahec
Copy link
Contributor Author

sarahec commented Sep 13, 2024 •
edited
Loading

Is this a SWIG 4 thing? We may be able to apply whatever upstream patch fixed it to the stable release.

It's more that SWIG 4.2.x, they had to make other changes to their .swig file so it would generate Darwin-buildable code. I tested the faiss nightly yesterday and they're considering cutting a new release (because I asked nicely).

P.S. I tried upstream patches and couldn't make it work.

@SomeoneSerge
Copy link
Contributor

@ofborg build faiss python3Packages.faiss

(needs a commit message in the ${attrPath}: ${message} format for automation)

@sarahec
Copy link
Contributor Author

sarahec commented Sep 16, 2024

@SomeoneSerge if faiss comes up with a new release (hopefully), I'll turn this into a pure upgrade, remove the patches, and mark it ready for review. If they don't, I expect to close this.

They have some patches in their history that we might cherry-pick to solve the bug, but I tried the likely candidates and it didn't work out.

@sarahec
Copy link
Contributor Author

sarahec commented Sep 18, 2024 •
edited
Loading

They expect to produce a release next week. If and when that happens, I'll update this and enter it for review.

@ofborg ofborg bot requested a review from SomeoneSerge September 26, 2024 00:17
@sarahec sarahec marked this pull request as ready for review October 4, 2024 22:47
@sarahec
Copy link
Contributor Author

sarahec commented Oct 4, 2024

@SomeoneSerge it's ready for review

@sarahec
Copy link
Contributor Author

sarahec commented Oct 4, 2024

@ofborg build faiss python3Packages.faiss

@sarahec sarahec changed the title DO NOT MERGE testing faiss nightly source as a prelude to getting an actual release faiss: 1.8.0->1.9.0 Oct 4, 2024
@sarahec
Copy link
Contributor Author

sarahec commented Oct 5, 2024

Passthru testing is failing due to the demo app it uses being unmaintained (and has been broken since at least 1.8.0). I'm going to remove it.

@sarahec sarahec force-pushed the faiss-darwin branch 2 times, most recently from fe9b27f to a4b5751 Compare October 5, 2024 16:28
@sarahec
Copy link
Contributor Author

sarahec commented Oct 5, 2024

Result of nixpkgs-review pr 341418 run on aarch64-darwin 1

3 packages marked as broken and skipped:
  • python312Packages.txtai
  • python312Packages.txtai.dist
  • sqlite-vss
28 packages built:
  • faiss
  • faiss.dist
  • open-webui
  • open-webui.dist
  • python311Packages.autofaiss
  • python311Packages.autofaiss.dist
  • python311Packages.colbert-ai
  • python311Packages.colbert-ai.dist
  • python311Packages.faiss
  • python311Packages.faiss.dist
  • python311Packages.pyannote-audio
  • python311Packages.pyannote-audio.dist
  • python311Packages.pytorch-metric-learning
  • python311Packages.pytorch-metric-learning.dist
  • python311Packages.txtai
  • python311Packages.txtai.dist
  • python312Packages.autofaiss
  • python312Packages.autofaiss.dist
  • python312Packages.colbert-ai
  • python312Packages.colbert-ai.dist
  • python312Packages.faiss
  • python312Packages.faiss.dist
  • python312Packages.pyannote-audio
  • python312Packages.pyannote-audio.dist
  • python312Packages.pytorch-metric-learning
  • python312Packages.pytorch-metric-learning.dist
  • whisper-ctranslate2
  • whisper-ctranslate2.dist

@sarahec
Copy link
Contributor Author

sarahec commented Oct 5, 2024

@ofborg build faiss

@sarahec sarahec force-pushed the faiss-darwin branch 2 times, most recently from 428a68b to 099fb81 Compare October 9, 2024 17:27
@sarahec
Copy link
Contributor Author

sarahec commented Oct 9, 2024

@SomeoneSerge welcome back. May I get your review?

@sarahec
Copy link
Contributor Author

sarahec commented Oct 9, 2024

Result of nixpkgs-review pr 341418 run on aarch64-darwin 1

3 packages marked as broken and skipped:
  • python312Packages.txtai
  • python312Packages.txtai.dist
  • sqlite-vss
22 packages built:
  • faiss
  • faiss.dist
  • open-webui
  • open-webui.dist
  • python311Packages.autofaiss
  • python311Packages.autofaiss.dist
  • python311Packages.colbert-ai
  • python311Packages.colbert-ai.dist
  • python311Packages.faiss
  • python311Packages.faiss.dist
  • python311Packages.pytorch-metric-learning
  • python311Packages.pytorch-metric-learning.dist
  • python311Packages.txtai
  • python311Packages.txtai.dist
  • python312Packages.autofaiss
  • python312Packages.autofaiss.dist
  • python312Packages.colbert-ai
  • python312Packages.colbert-ai.dist
  • python312Packages.faiss
  • python312Packages.faiss.dist
  • python312Packages.pytorch-metric-learning
  • python312Packages.pytorch-metric-learning.dist

@sarahec
Copy link
Contributor Author

sarahec commented Oct 9, 2024

Result of nixpkgs-review pr 341418 run on x86_64-linux 1

3 packages marked as broken and skipped:
  • python312Packages.txtai
  • python312Packages.txtai.dist
  • sqlite-vss
2 packages failed to build:
  • python311Packages.pyannote-audio
  • python311Packages.pyannote-audio.dist
28 packages built:
  • faiss
  • faiss.dist
  • faissWithCuda
  • faissWithCuda.dist
  • open-webui
  • open-webui.dist
  • python311Packages.autofaiss
  • python311Packages.autofaiss.dist
  • python311Packages.colbert-ai
  • python311Packages.colbert-ai.dist
  • python311Packages.faiss
  • python311Packages.faiss.dist
  • python311Packages.pytorch-metric-learning
  • python311Packages.pytorch-metric-learning.dist
  • python311Packages.txtai
  • python311Packages.txtai.dist
  • python312Packages.autofaiss
  • python312Packages.autofaiss.dist
  • python312Packages.colbert-ai
  • python312Packages.colbert-ai.dist
  • python312Packages.faiss
  • python312Packages.faiss.dist
  • python312Packages.pyannote-audio
  • python312Packages.pyannote-audio.dist
  • python312Packages.pytorch-metric-learning
  • python312Packages.pytorch-metric-learning.dist
  • whisper-ctranslate2
  • whisper-ctranslate2.dist

SomeoneSerge
SomeoneSerge reviewed Oct 9, 2024
View reviewed changes
pkgs/development/libraries/science/math/faiss/default.nix
Comment on lines -132 to -133
tests = {
runDemos =
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be nice to have more thorough integration testing in the long term, but I guess right now this is more of a maintenance burden

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed. The demos don't seem to go through their CI, alas.

@ofborg ofborg bot requested a review from SomeoneSerge October 9, 2024 21:04
@SomeoneSerge SomeoneSerge merged commit ed42e8f into NixOS:master Oct 9, 2024
32 of 33 checks passed
@sarahec sarahec mentioned this pull request Oct 11, 2024
Closed
@trofi
Copy link
Contributor

trofi commented Oct 12, 2024

The removal of passthru in faiss:

  passthru = {
    inherit cudaSupport cudaPackages pythonSupport;

broke pass thru attributes of python bindings:

$ nix repl -f.
nix-repl> python3Packages.faiss.passthru
  cudaPackages = «error: attribute 'cudaPackages' missing»;
  cudaSupport = «error: attribute 'cudaSupport' missing»;
  pythonSupport = «error: attribute 'pythonSupport' missing»;

Should those be removed from bindings? Or restored in the library?

@trofi
Copy link
Contributor

trofi commented Oct 12, 2024

Proposed simple attribute restore as:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SomeoneSerge SomeoneSerge SomeoneSerge approved these changes

Assignees
No one assigned
Labels
6.topic: darwin Running or building packages on Darwin 10.rebuild-darwin: 11-100 10.rebuild-linux: 11-100
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@sarahec @emilazy @SomeoneSerge @trofi