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

Import bitsandbytes only if it is used #546

Merged
merged 2 commits into from
Jan 6, 2023
Merged

Conversation

borzunov
Copy link
Member

@borzunov borzunov commented Jan 6, 2023

When bitsandbytes is installed, hivemind always tries to import it even if it doesn't work. This leads to a number of undesirable behaviors:

  • import hivemind fails if bitsandbytes is installed but fails to resolve CUDA dynamic libraries or load its own dynamic libraries. This happens all the time:
    • on CPU-only hosts with CUDA installed (e.g., Colabs)
    • on hosts with misconfigured CUDA (e.g., most nora envs)
    • on macOS (even when the daemon is compiled correctly and it should work), etc.
  • Users see the irrelevant bitsandbytes welcome message even if they don't use it. This message is currently impossible to suppress. Multiple users has reported that this is not expected when they import hivemind/petals.

This PR fixes it, so now hivemind's behavior matches with, e.g., HF transformers (import bitsandbytes only if it's necessary).

@borzunov borzunov requested a review from mryab January 6, 2023 07:00
@borzunov borzunov force-pushed the optional-bnb-import branch 3 times, most recently from b145a36 to 35bc63a Compare January 6, 2023 07:12
@codecov
Copy link

codecov bot commented Jan 6, 2023

Codecov Report

Merging #546 (e6b1740) into master (41a2d06) will decrease coverage by 0.01%.
The diff coverage is 66.66%.

@@            Coverage Diff             @@
##           master     #546      +/-   ##
==========================================
- Coverage   75.96%   75.94%   -0.02%     
==========================================
  Files          81       81              
  Lines        7979     7978       -1     
==========================================
- Hits         6061     6059       -2     
- Misses       1918     1919       +1     
Impacted Files Coverage Δ
hivemind/compression/quantization.py 94.82% <66.66%> (-0.05%) ⬇️
hivemind/moe/server/connection_handler.py 46.87% <0.00%> (-1.05%) ⬇️
hivemind/moe/server/runtime.py 69.16% <0.00%> (-0.84%) ⬇️
hivemind/moe/server/server.py 43.71% <0.00%> (-0.55%) ⬇️
hivemind/dht/protocol.py 93.15% <0.00%> (+0.91%) ⬆️

@@ -9,13 +9,11 @@
import numpy as np
import torch

if importlib.util.find_spec("bitsandbytes") is not None:
Copy link
Member

Choose a reason for hiding this comment

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

importlib.util is no longer used, so its import can be removed

@@ -132,6 +130,9 @@ class BlockwiseQuantization(Quantization):
def quantize(
self, tensor: torch.Tensor, allow_inplace: bool = False
) -> Tuple[np.ndarray, Tuple[np.ndarray, np.ndarray]]:
# This runs actual import only on the 1st call, copies references after that
from bitsandbytes.functional import quantize_blockwise
Copy link
Member

Choose a reason for hiding this comment

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

Since this is no longer under a conditional import (and will thus cause ImportError instead of NameError), can we wrap this statement instead of the quantize_blockwise call below with try-except? The same applies to extract

@borzunov borzunov changed the title Import bitsandbytes only if it's necessary Import bitsandbytes only if it is used Jan 6, 2023
@borzunov borzunov merged commit e18868f into master Jan 6, 2023
@borzunov borzunov deleted the optional-bnb-import branch January 6, 2023 08:12
mryab pushed a commit that referenced this pull request Jan 6, 2023
When bitsandbytes is installed, hivemind always tries to import it even if it doesn't work. This leads to a number of undesirable behaviors:

- `import hivemind` fails if bitsandbytes is installed but fails to resolve CUDA dynamic libraries or load its own dynamic libraries. This happens all the time:
    - on CPU-only hosts with CUDA installed (e.g., Colabs)
    - on hosts with misconfigured CUDA (e.g., most nora envs)
    - on macOS (even when the daemon is compiled correctly and it should work), etc.
- Users see the irrelevant bitsandbytes welcome message even if they don't use it. This message is currently impossible to suppress. Multiple users has reported that this is not expected when they import hivemind/petals.

This PR fixes it, so now hivemind's behavior matches with, e.g., HF transformers (import bitsandbytes only if it's necessary).
mryab pushed a commit that referenced this pull request Jan 7, 2023
When bitsandbytes is installed, hivemind always tries to import it even if it doesn't work. This leads to a number of undesirable behaviors:

- `import hivemind` fails if bitsandbytes is installed but fails to resolve CUDA dynamic libraries or load its own dynamic libraries. This happens all the time:
    - on CPU-only hosts with CUDA installed (e.g., Colabs)
    - on hosts with misconfigured CUDA (e.g., most nora envs)
    - on macOS (even when the daemon is compiled correctly and it should work), etc.
- Users see the irrelevant bitsandbytes welcome message even if they don't use it. This message is currently impossible to suppress. Multiple users has reported that this is not expected when they import hivemind/petals.

This PR fixes it, so now hivemind's behavior matches with, e.g., HF transformers (import bitsandbytes only if it's necessary).

(cherry picked from commit e18868f)
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

Successfully merging this pull request may close these issues.

None yet

2 participants