python3Packages.mamba-ssm: init at 2.2.2 #341195
Conversation
cfhammill
force-pushed
the
init-mamba
branch
2 times, most recently
from
513cd1d to
64bb8e1
Compare
ofborg
bot
added
8.has: package (new)
wegank
added
the
12.approvals: 1
cfhammill
force-pushed
the
init-mamba
branch
from
64bb8e1 to
7679ebb
Compare
|
thanks @samuela. I've pushed a few metadata fixes, are you interested in being added as a co-maintainer? |
|
@cfhammill unfortunately i don't have bandwidth to take on new maintainer duties atm |
|
ok, no problem, thanks @samuela |
wegank
removed
the
12.approvals: 1
| ]; | ||
|
|
||
| pythonImportsCheck = [ "causal_conv1d" ]; | ||
|
|
There was a problem hiding this comment.
Could you enable tests with pytestCheckHook?
https://github.com/NixOS/nixpkgs/blob/master/doc/languages-frameworks/python.section.md#using-pytestcheckhook-using-pytestcheckhook
There was a problem hiding this comment.
the most common use case would be to build this with CUDA, I know there has been work done to enable gpu tests but I'm not sure how to do it. As mentioned in the PR description I haven't tested on cpu or with rocm, I would rather mark broken if the user doesn't have cuda enabled.
There was a problem hiding this comment.
I've checked, neither introduced packages actually supports cpu only execution, so I've marked the package as broken without CUDA. I'm going to leave tests disabled because I don't have a nixos machine with cuda to experiment on.
There was a problem hiding this comment.
That seems fair enough; shall we leave a comment explaining the situation?
There was a problem hiding this comment.
I've added a comment indicating the tests are disabled.
| } // lib.optionalAttrs cudaSupport { CUDA_HOME = "${lib.getDev cudaPackages.cuda_nvcc}"; }; | ||
|
|
||
| pythonImportsCheck = [ "mamba_ssm" ]; | ||
|
|
cfhammill
force-pushed
the
init-mamba
branch
2 times, most recently
from
19c4d66 to
2261a79
Compare
| }; | ||
|
|
||
| build-system = [ | ||
| ninja |
There was a problem hiding this comment.
Could you explicitly add setuptools, as this package depends on implicitly propagated setuptools from somewhere?
I'm sorry to say this is also in the future, but please add torch as well since it will not be able to access the runtime dependencies at build time.
These changes will reduce build failures caused by other packages.
If you are interested, the following issue may be helpful. #272178
There was a problem hiding this comment.
I'm out of the loop, how does mamba-ssm use torch at build time? build platform's torch at build time?
There was a problem hiding this comment.
Ah, OK. I checked, it's just a "normal" cpp_extension and ofc it's doing import torch in setup.py which screws up all of the cross stuff.
Can't judge about semantics yet, but the way we implement build-system (mapping it into nativeBuildInputs) this is obviously not the torch we want to link against, because we want to link torch from buildInputs
There was a problem hiding this comment.
EDIT: As a matter of fact, semantically, torch from nativeBuildInputs would be correct if we actually made it work like a compiler. Obviously, though, it does not implement the promise
There was a problem hiding this comment.
I agree that build-system is incomplete since it does not support cross compiling.
However, python modules will separate build and runtime dependencies, and dependencies will no longer be available during build.
So, in this case, we need to add torch not only to dependencies but also to build-system.
There was a problem hiding this comment.
ok great, then I think we're good to go.
cfhammill
force-pushed
the
init-mamba
branch
from
2261a79 to
80327a7
Compare
|
Unsuccessful checks look unrelated to this PR. @natsukium and @SomeoneSerge just let me know if we should have |
cfhammill
force-pushed
the
init-mamba
branch
from
80327a7 to
4337d63
Compare
|
Rebased and ready to go @natsukium and @SomeoneSerge |
|
@SomeoneSerge @samuela @natsukium, thanks for the reviews everyone! If someone could pull that would be great. |
Description of changes
Adds python3Packages.mamba-ssm and its dependency python3Packages.causal-conv1d.
This PR is mostly a proof of concept, I have only tested on python311 with cuda, I have not attempted to figure out rocm, or cpu only execution. Tests green for me after loading with
cloning the mamba and causal-conv1d repos and running the test suites.
I'm not interested in being the sole maintainer of this, so if any of the cuda team wants to join and I can add you to the maintainer list.
cc: @ConnorBaker @samuela @SomeoneSerge @bcdarwin
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.