Skip to content

Pin the eager decoder in the AuxK test so it runs anywhere - #142

Open
alepot55 wants to merge 1 commit into
EleutherAI:mainfrom
alepot55:pr-auxk-test
Open

Pin the eager decoder in the AuxK test so it runs anywhere#142
alepot55 wants to merge 1 commit into
EleutherAI:mainfrom
alepot55:pr-auxk-test

Conversation

@alepot55

Copy link
Copy Markdown

What

test_auxk_loss_does_not_double_count_b_dec builds CPU tensors, and its docstring says it "runs on CPU using the eager decoder fallback, so it requires no GPU". Nothing makes that true. decoder_impl is bound at import time in sparsify/utils.py and prefers the Triton kernel whenever sparsify.xformers imports, which it does whenever Triton is installed, GPU or not.

So the test fails on both kinds of machine, for the same reason:

  • CPU-only box: RuntimeError: 0 active drivers ([]). There should only be one.
  • GPU box: ValueError: Pointer argument (at 0) cannot be accessed from Triton (cpu tensor?)

pytest is red on a fresh clone either way. Setting SPARSIFY_DISABLE_TRITON=1 works around it, but the test should not need the caller to know that.

Fix

Patch decoder_impl for the duration of the test with monkeypatch, and call eager_decode directly for the reference value.

After

On an A100, pytest on main goes from 1 failed, 10 passed to 11 passed.

The test builds CPU tensors and its docstring says it uses the eager decoder
fallback, but nothing made that true: `decoder_impl` is bound at import time
and prefers the Triton kernel, which rejects CPU tensors. The test therefore
fails on any machine where Triton imports, GPU or not, so `pytest` is red on a
fresh clone.

Patch `decoder_impl` for the duration of the test and call `eager_decode` for
the reference value.
@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

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.

2 participants