Skip to content

Fix negative inter-op count in compute pool - #124960

Draft
DebadityaHait wants to merge 1 commit into
tensorflow:masterfrom
DebadityaHait:fix-negative-inter-op-compute-pool
Draft

Fix negative inter-op count in compute pool#124960
DebadityaHait wants to merge 1 commit into
tensorflow:masterfrom
DebadityaHait:fix-negative-inter-op-compute-pool

Conversation

@DebadityaHait

@DebadityaHait DebadityaHait commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Treat negative inter_op_parallelism_threads values passed to the process-wide compute pool as unspecified, using the existing default path.
  • Add regression coverage that ComputePool creates a valid pool for -1.

Addresses #124954.

Validation

  • git diff --check
  • CI: bazel test --config=linux -k //tensorflow/core/common_runtime:process_util_test

@google-ml-butler google-ml-butler Bot added the size:S CL Change Size: Small label Aug 9, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request updates InitComputePool to fall back to the default number of threads when inter_op_parallelism_threads is less than or equal to zero, and adds a unit test to verify this behavior. The feedback suggests strengthening the test assertion to ensure the thread pool size matches DefaultNumInterOpThreads() exactly, rather than just verifying it is greater than zero.

opts.config.set_inter_op_parallelism_threads(-1);

thread::ThreadPool* pool = ComputePool(opts);
EXPECT_GT(pool->NumThreads(), 0);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The assertion EXPECT_GT(pool->NumThreads(), 0) is very weak and only verifies that the pool has at least one thread. Since the goal of passing a negative value is to fall back to the default number of threads, we should explicitly assert that pool->NumThreads() equals DefaultNumInterOpThreads(). This provides a much stronger guarantee that the default path was correctly taken.

Suggested change
EXPECT_GT(pool->NumThreads(), 0);
EXPECT_EQ(pool->NumThreads(), DefaultNumInterOpThreads());

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:S CL Change Size: Small

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants