Don't require scipy for regular use #948
Conversation
|
Thanks for raising this, makes sense! I need to discuss this with the other maintainers first. I put it on my list. Will get back to you. |
akx
force-pushed
the
no-hard-scipy-requirement
branch
from
56e47d7 to
edc83e2
Compare
akx
force-pushed
the
no-hard-scipy-requirement
branch
from
edc83e2 to
996f23f
Compare
|
Rebased post #488. |
| install_requires=['torch', 'numpy'], | ||
| extras_require={ | ||
| 'benchmark': ['pandas', 'matplotlib'], | ||
| 'test': ['scipy'], |
There was a problem hiding this comment.
Is create_normal_map only used for testing? Otherwise the name might be confusing?
There was a problem hiding this comment.
The only call to the function is in a test (and it's mentioned in a couple copy-pasted comments too):
(bitsandbytes) ~/b/bitsandbytes (main) $ ag create_normal_map
tests/test_functional.py
2356: code = F.create_normal_map()
bitsandbytes/nn/modules.py
306: Implementation of the NF4 data type in bitsandbytes can be found in the `create_normal_map` function in
bitsandbytes/functional.py
236:def create_normal_map(offset=0.9677083, use_extra_value=True):
847: Implementation of the NF4 data type in bitsandbytes can be found in the `create_normal_map` function
I can't find other relevant references to it across all of GitHub either – and anyway, the function is still there for downstream users who might need it, they'll just need scipy installed.
| extras_require={'benchmark': ['pandas', 'matplotlib']}, | ||
| install_requires=['torch', 'numpy'], | ||
| extras_require={ | ||
| 'benchmark': ['pandas', 'matplotlib'], |
There was a problem hiding this comment.
Are these runtime dependencies at all? The seem to be used only in scripts and are not part of the wheel? Maybe the benchmark extra is not needed at all?
There was a problem hiding this comment.
The idea is that when desiring to run the benchmark scripts, you'd do
pip install -e .[benchmark]
to get the regular requirements plus the things required for benchmarking, or if you want that and the test deps,
pip install -e .[benchmark,test]
There was a problem hiding this comment.
Doesn’t this also make it part of the public metadata, e.g “pip install bitsandbytes[benchmark]” ?
|
Checked with Tim. He agrees that this makes sense and sees no issue with it. Thanks for raising this and taking the initiative to provide a fix! |
Titus-von-Koeller
merged commit 32be289
into
bitsandbytes-foundation:main
Refs #488.
The only use for
scipy.stats.normiscreate_normal_map, so it probably does not need to be a hard requirement for any and all users of the library.Many users will of course have it installed anyway.