Fix batched silhouette score reduction aliasing - #2422
Draft
csadorf wants to merge 5 commits into
Draft
Conversation
csadorf
added
bug
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
This comment has been minimized.
This comment has been minimized.
csadorf
force-pushed
the
fix-batched-silhouette-reduction-aliasing
branch
from
August 6, 2026 18:08
f8c9939 to
6ec7aeb
Compare
This comment has been minimized.
This comment has been minimized.
csadorf
changed the title
This comment has been minimized.
This comment has been minimized.
csadorf
force-pushed
the
fix-batched-silhouette-reduction-aliasing
branch
from
August 7, 2026 14:57
6ec7aeb to
d24aff2
Compare
This comment has been minimized.
This comment has been minimized.
csadorf
force-pushed
the
fix-batched-silhouette-reduction-aliasing
branch
from
August 7, 2026 15:48
d24aff2 to
77aaa23
Compare
This comment has been minimized.
This comment has been minimized.
Contributor
Author
|
The negative-control run behaved as expected. With the presumed fix reverted, This confirms that the stress test detects the unfixed behavior in the target environment. The next step is to restore the fix and rerun the same matrix; the A100/CUDA 12.2 test should then pass. The separate NN-descent recall failure on L4 appears unrelated. |
csadorf
force-pushed
the
fix-batched-silhouette-reduction-aliasing
branch
from
August 10, 2026 15:44
77aaa23 to
20bd669
Compare
This comment has been minimized.
This comment has been minimized.
csadorf
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stores the row-wise minimum for batched silhouette scores in a separate device vector, preventing the reduction output from overwriting matrix elements that may still be read.
Adds
BatchedMatchesNonBatchedAcrossMetricsAndChunkSizescoverage for four distance metrics, three chunk sizes, and both default and stream-pool handles. The test uses seed170, which reproduced the bug during concurrent A100 testing. It remains deterministic, single-process coverage and is not guaranteed to reproduce the scheduling-sensitive race.Validation
Regression validation used the temporary
BATCHED_SILHOUETTE_CONCURRENCY_TESTadded inef922834, originallye20bfffcbefore the rebase. The temporary test has since been reverted.As a negative control, reverting the fix while retaining the test caused the A100/CUDA 12.2 CI job to fail: 12 of 512 worker seeds produced 22 batched/non-batched mismatches above the
1e-4tolerance. This confirms that the stress workload detects the unfixed reduction-aliasing behavior in the target environment.Closes #2432