Skip to content

Feat/precomputed noise pool - #518

Draft
GOavi101 wants to merge 7 commits into
torch-spyre:mainfrom
GOavi101:feat/precomputed-noise-pool
Draft

Feat/precomputed noise pool #518
GOavi101 wants to merge 7 commits into
torch-spyre:mainfrom
GOavi101:feat/precomputed-noise-pool

Conversation

@GOavi101

@GOavi101 GOavi101 commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Description

  • Port Holtz async Exp(1) log-noise ring buffer from sendnn-inference#1046: background thread fills noise; host sampling consumes from the pool.
  • Sample on TP rank 0 and broadcast token ids; use log-space Gumbel (argmax(logits - log_noise)) so the hot path avoids softmax.
  • Wire via build_spyre_sampler in the Spyre model runner; document host-sampler config; add unit tests for the ring buffer, sampler, and top-k/top-p path.

docs in docs/user_guide/configuration.md.

Related Issues

Relates to host sampling performance / sendnn-style Exp(1) pool reuse.

Test Plan

  • Unit tests: pytest tests/test_spyre_noise_pool.py -q (pool fill, slice, seeded path, opt-in wiring)
  • Manual (Spyre host), compare sampler timing with pool on vs off:
    SPYRE_USE_NOISE_POOL=1 SPYRE_SAMPLER_TIMING=50 \
      python examples/offline_inference/torch_spyre_inference.py
    # use temperature > 0; greedy does not exercise the pool

Checklist

  • I have read the contributing guidelines
  • My code follows the project's code style (run bash format.sh)
  • I have added tests for my changes (if applicable)
  • I have updated the documentation (if applicable)
  • My commits include a Signed-off-by: line (DCO compliance)

@github-actions

Copy link
Copy Markdown
Contributor

👋 Hi! Thank you for contributing.
Just a reminder: Make sure that your code passes all the linting checks, otherwise your PR won't be able to be merged. To do so, run ./format.sh.
Now you are good to go 🚀.

We also recommend installing prek and configuring it to check your code before every local commit.

@github-actions

github-actions Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

📖 This PR includes doc changes. Download the preview (built from 2f8165a), or build locally:

uv pip install -r docs/requirements-docs.txt
mkdocs serve

@GOavi101
GOavi101 force-pushed the feat/precomputed-noise-pool branch from ce1e679 to b07b686 Compare August 13, 2026 10:14

_USE_NOISE_POOL = os.environ.get("SPYRE_USE_NOISE_POOL", "0") == "1"
_NOISE_POOL_MULTIPLIER = int(os.environ.get("SPYRE_NOISE_POOL_MULTIPLIER", "32"))
_SAMPLER_TIMING = int(os.environ.get("SPYRE_SAMPLER_TIMING", "0"))

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

a few thoughts:

  1. SPYRE_SAMPLER_TIMING smells like something that we should be using the profiler for instead of injecting into a production code path
  2. We need to do config properly here- at the minimum the envs.py approach that puts all the config levers in one place with documentation, caching, and override semantics is required.
  3. This feels like running before we crawl, it's an optimization for noise generation for sampling on a specific cpu platform but we haven't even done the work yet to put the rest of the sampling into a torch.compiled path on the spyre devices. The model / sampler split on sendnn-inference was a limitation that we couldn't work around that no longer exists here

@GOavi101 GOavi101 Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Agree on sequencing. this is only an opt-in host stopgap for slow Exp(1) on s390x while sampling is still on CPU — it doesn’t move sampling onto Spyre.

for that few pain point i have analyzed:

  1. argmax— CPU fallback
  2. topk— native but unusable (k≤4, fp16 indices wrong past 2048)
  3. cumsum— CPU fallback (top-p)
  4. sort— unsupported (top-p / joint top-k+top-p)
  5. scatter / scatter_add / gather / index_put_— incomplete/broken (top-p, penalties, logprobs)
  6. exponential/ RNG— CPU fallback (temp / random sample)
    CC: @dilipgb @rishikakedia

@GOavi101
GOavi101 marked this pull request as ready for review August 14, 2026 03:32
@GOavi101
GOavi101 requested a review from a team as a code owner August 14, 2026 03:32
@GOavi101
GOavi101 marked this pull request as draft August 14, 2026 03:42
@GOavi101
GOavi101 requested a review from joerunde August 14, 2026 08:03
@GOavi101
GOavi101 marked this pull request as ready for review August 14, 2026 11:04

@tdoublep tdoublep left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I am concerned that this approach will change the sampling statistics in strange and ways that are hard to interpret. I would prefer to hold off on merging.

@GOavi101
GOavi101 marked this pull request as draft August 17, 2026 16:59
Avishek Goswami added 3 commits August 20, 2026 17:46
Port sendnn-inference#1046 sampling stages to spyre-inference:
1) async Exp(1) log-noise ring buffer on a background thread
2) TP rank-0 sample + broadcast when logits are on CPU
3) log-space Gumbel argmax(logits - log_noise) to skip softmax

Wire SpyreSampler into TorchSpyreModelRunner and add unit tests.

Signed-off-by: Avishek Goswami <avishek.goswami@ibm.com>
Port the vLLM-style envs module from the precomputed-noise-pool work and
wire SPYRE_ASYNC_NOISE_SCALE into Holtz's async ring buffer. The static
Exp(1) pool is superseded by the background refill path.

Signed-off-by: Avishek Goswami <avishek.goswami@ibm.com>
Async ring buffer is the host noise path; remove leftover mentions of the
superseded static pool.

Signed-off-by: Avishek Goswami <avishek.goswami@ibm.com>
@GOavi101
GOavi101 force-pushed the feat/precomputed-noise-pool branch from 0303451 to f51eb4c Compare August 20, 2026 12:21
Avishek Goswami added 4 commits August 20, 2026 17:55
Drop the envs/noise-scale extras that are not in Holtz's PR. Match his
SpyreSampler vocab lookup, and wire with is_vllm_config_compatible
fallback to the default Sampler (same as SpyreCausalLM).

Signed-off-by: Avishek Goswami <avishek.goswami@ibm.com>
Address review: keep all sampler knobs in a vLLM-style envs module
(docs, defaults, cache, is_set overrides). Wire SPYRE_USE_SPYRE_SAMPLER
and SPYRE_ASYNC_NOISE_SCALE; do not inject production-path timing —
use the Kineto profiler instead.

Signed-off-by: Avishek Goswami <avishek.goswami@ibm.com>
Under the Spyre plugin, default-device Spyre tensors hang on background
refill and deadlock borrow_rows after the first wrap.

Signed-off-by: Avishek Goswami <avishek.goswami@ibm.com>
Under the Spyre plugin the producer thread was dying on Torch fill_/
exponential_ (absent from the pytest-timeout dump), so borrow_rows
deadlocked after the first wrap. Keep a NumPy backing store shared with
a Torch CPU view; surface producer failures instead of waiting forever.

Signed-off-by: Avishek Goswami <avishek.goswami@ibm.com>
@dmholtz

dmholtz commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

As the scope of this PR changed towards the direction of torch-spyre/sendnn-inference#1046:

I agreed with @tdoublep to provide an analogous solution to sendnn-inference here; however I think PR 1046 should not be mirrored 1:1 here. For example, the softmax function could be properly computed on device with this stack. There is currently no rush to address the sampler before batching support lands, so I will provide a draft of that idea in about two weeks.

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.

4 participants