Skip to content

Simplify GPU quantile tests and tighten sketch container ownership#12160

Draft
RAMitchell wants to merge 12 commits intodmlc:masterfrom
RAMitchell:quantile-gpu-test-strategy
Draft

Simplify GPU quantile tests and tighten sketch container ownership#12160
RAMitchell wants to merge 12 commits intodmlc:masterfrom
RAMitchell:quantile-gpu-test-strategy

Conversation

@RAMitchell
Copy link
Copy Markdown
Member

Summary

This continues the GPU quantile test rewrite and tightens the GPU SketchContainer
interface so the sketch owns more of its own sizing and maintenance behavior.

What Changed

  • simplify the low-level GPU sketch tests in tests/cpp/common/test_quantile.cu
    • use direct SketchContainer::Push(...) input for push/prune/merge coverage
    • remove tests that belonged at the wrapper/cut layer instead of the sketch layer
    • keep explicit duplicate/rank coverage for Push(...)
  • align GPU property-style cut tests with the shared CPU helper layer
    • shared container cases
    • shared cut invariant validation
  • tighten the GPU sketch container interface
    • SketchContainer::Push(...) now computes its own batch-local cut layout
    • internal maintenance prune uses a per-feature bound
    • remove external post-batch prune from QuantileDMatrix
  • remove redundant GPU wrapper tests from tests/cpp/common/test_hist_util.cu
  • simplify categorical dedup bookkeeping by updating the new column scan buffer directly

Testing

Built testxgboost with CUDA and ran focused GPU coverage, including:

  • GPUQuantile.*
  • GPUQuantileProperty.Invariants
  • MGPUQuantileTest.SameOnAllWorkers
  • focused remaining HistUtil GPU tests

Local result:

  • focused GPU low-level slice passed
  • SameOnAllWorkers still skips locally without federated/NCCL support

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors GPU quantile sketch tests and tightens the GPU SketchContainer interface so the container computes its own per-batch cut layout and owns more of its maintenance behavior.

Changes:

  • Simplify/realign GPU quantile tests to exercise SketchContainer::Push/Prune/Merge/AllReduce more directly and share CPU/GPU cut invariant validation via helpers.
  • Update GPU sketch container API so Push(...) computes batch-local cut pointers internally; remove redundant external prune in QuantileDMatrix.
  • Simplify categorical dedup bookkeeping by updating the column scan buffer directly; remove redundant GPU wrapper tests from test_hist_util.cu.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tests/cpp/common/test_quantile.h Updates helper header guard and seed generation used by quantile tests.
tests/cpp/common/test_quantile.cu Major GPU quantile test rewrite: new synthetic batch helpers + property-style tests; updated calls to new SketchContainer::Push signature.
tests/cpp/common/test_quantile.cc Replaces local container-invariant logic with shared ValidateContainerCuts helper.
tests/cpp/common/test_quantile_helpers.h Adds shared ValidateContainerCuts(...) helper used by both CPU/GPU tests.
tests/cpp/common/test_hist_util.cu Removes redundant DeviceSketch wrapper tests; updates categorical dedup test for new API.
src/data/quantile_dmatrix.cu Removes per-batch external prune now handled internally by the sketch container.
src/common/quantile.cuh Tightens SketchContainer::Push signature; renames intermediate sizing helper to per-feature semantics.
src/common/quantile.cu Implements internal batch-local cut pointer construction and prunes using a per-feature bound.
src/common/hist_util.cuh Removes external cut-pointer plumbing; relies on SketchContainer::Push to compute cut layout.
src/common/hist_util.cu Simplifies categorical dedup to update column scan only; updates weighted sketch path to new Push API.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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