-
Notifications
You must be signed in to change notification settings - Fork 660
[CI] 【Hackathon 9th Sprint No.20】NO.20 功能模块单测补充 #5065
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
[CI] 【Hackathon 9th Sprint No.20】NO.20 功能模块单测补充 #5065
Conversation
This commit adds unit tests for the sampler helpers, covering guided decoding and speculative flows. It includes various test cases to ensure the correct functionality of the sampling methods and processors.
|
Thanks for your contribution! |
There was a problem hiding this 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 adds comprehensive unit tests for the sampler module (fastdeploy/model_executor/layers/sample/sampler.py) as part of Hackathon 9th Sprint No.20. The tests cover guided decoding, speculative sampling, and MTP sampling flows, achieving 82% code coverage.
Key Changes:
- New test file with 435 lines covering
Sampler,SpeculativeSampler,MTPSampler, andGuidedDecodingclasses - Extensive mocking infrastructure for platform detection and CUDA operations
- Test fixtures for various sampling scenarios and configurations
Comments suppressed due to low confidence (1)
tests/model_executor/test_sampler.py:260
- Call to GuidedDecoding.init with too few arguments; should be no fewer than 1.
guided = sampler_module.GuidedDecoding()
tests/model_executor/test_sampler.py
Outdated
| def test_sampler_gather_logprobs_variants(set_platform): | ||
| set_platform("cuda") | ||
| sampler = sampler_module.Sampler() |
Copilot
AI
Nov 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Creating a Sampler() without an fd_config parameter will cause an AttributeError because GuidedDecoding initialization requires fd_config.scheduler_config.max_num_seqs. You should pass a mock config object using fd_config_factory.
| def test_sampler_gather_logprobs_variants(set_platform): | |
| set_platform("cuda") | |
| sampler = sampler_module.Sampler() | |
| def test_sampler_gather_logprobs_variants(set_platform, fd_config_factory): | |
| set_platform("cuda") | |
| sampler = sampler_module.Sampler(fd_config=fd_config_factory()) |
tests/model_executor/test_sampler.py
Outdated
|
|
||
| guided.add_logits_processor(0, None) | ||
| assert 0 not in guided.logits_processor | ||
| with pytest.raises(ValueError): |
Copilot
AI
Nov 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test expects a ValueError to be raised when calling _accept_token with an invalid index (99), but the actual implementation in sampler.py would raise an AttributeError (attempting to access attributes on None) or IndexError (index out of bounds), not ValueError. Either update the assertion to expect the correct exception type, or verify what exception the code actually raises.
| with pytest.raises(ValueError): | |
| with pytest.raises(IndexError): |
tests/model_executor/test_sampler.py
Outdated
| def test_sampler_forward_intel_hpu_path(set_platform): | ||
| set_platform("intel_hpu") | ||
| sampler = sampler_module.Sampler() |
Copilot
AI
Nov 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Creating a Sampler() without an fd_config parameter will cause an AttributeError because GuidedDecoding initialization requires fd_config.scheduler_config.max_num_seqs. You should pass a mock config object using fd_config_factory.
| def test_sampler_forward_intel_hpu_path(set_platform): | |
| set_platform("intel_hpu") | |
| sampler = sampler_module.Sampler() | |
| def test_sampler_forward_intel_hpu_path(set_platform, fd_config_factory): | |
| set_platform("intel_hpu") | |
| cfg = fd_config_factory() | |
| sampler = sampler_module.Sampler(fd_config=cfg) |
| class _FakePlatform: | ||
| def __init__(self, kind: str = "cuda"): | ||
| self.kind = kind | ||
|
|
||
| def is_cuda(self): # noqa: D401 - simple boolean helpers | ||
| return self.kind == "cuda" | ||
|
|
||
| def is_xpu(self): | ||
| return self.kind == "xpu" | ||
|
|
||
| def is_iluvatar(self): | ||
| return False | ||
|
|
||
| def is_gcu(self): | ||
| return False | ||
|
|
||
| def is_dcu(self): | ||
| return False | ||
|
|
||
| def is_maca(self): | ||
| return False | ||
|
|
||
| def is_intel_hpu(self): | ||
| return self.kind == "intel_hpu" |
Copilot
AI
Nov 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The _FakePlatform class lacks a docstring explaining its purpose. Consider adding a class-level docstring to document that this is a test fixture for mocking different hardware platforms (CUDA, XPU, Intel HPU, etc.).
| class _RecordingProcessor(sampler_module.LogitsProcessorBase): | ||
| def __init__(self, enable_reasoning: bool = False): | ||
| super().__init__(enable_reasoning=enable_reasoning) | ||
| self.accepted_tokens: list[int] = [] | ||
| self.token_bitmask = None | ||
| self.mask_fills: list[int] = [] | ||
| self.applied = 0 | ||
| self.terminated = False | ||
|
|
||
| def allocate_token_bitmask(self): | ||
| return paddle.zeros([2, 3], dtype="float32") | ||
|
|
||
| def fill_token_bitmask(self, token_bitmask, idx): | ||
| token_bitmask[idx, :] = idx + 1 | ||
| self.mask_fills.append(idx) | ||
|
|
||
| def apply_token_mask(self, logits, token_bitmask, indices=None): | ||
| self.applied += 1 | ||
| masked = logits.clone() | ||
| if indices: | ||
| for idx in indices: | ||
| masked[idx] = 0 | ||
| return masked | ||
|
|
||
| def accept_token(self, token): | ||
| self.accepted_tokens.append(int(token)) | ||
|
|
||
| def is_terminated(self): | ||
| return self.terminated | ||
|
|
||
| def reset(self): | ||
| self.accepted_tokens.clear() | ||
|
|
||
| def copy(self): # pragma: no cover - unused helper | ||
| return _RecordingProcessor() |
Copilot
AI
Nov 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The _RecordingProcessor class lacks a docstring. Consider adding a class-level docstring to document that this is a test mock for recording interactions with the LogitsProcessorBase interface.
| class _AddOffsetProcessor: | ||
| def __init__(self, delta: float = 1.0): | ||
| self.delta = delta | ||
|
|
||
| def apply(self, logits): | ||
| return logits + self.delta |
Copilot
AI
Nov 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The _AddOffsetProcessor class lacks a docstring. Consider adding documentation to explain its purpose as a test helper for adding offsets to logits.
| class _FakeEarlyStopper: | ||
| def __init__(self): | ||
| self.initialized_with = None | ||
| self.process_calls: list[tuple] = [] | ||
|
|
||
| def initialize(self, max_num_seqs, config): | ||
| self.initialized_with = (max_num_seqs, config) | ||
|
|
||
| def process(self, probs, next_tokens, stop_flags): | ||
| self.process_calls.append((probs, next_tokens, stop_flags)) |
Copilot
AI
Nov 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The _FakeEarlyStopper class lacks a docstring. Consider adding documentation to explain its purpose as a mock for the early stopping functionality.
tests/model_executor/test_sampler.py
Outdated
| def test_guided_decoding_tracks_processors_and_reasoning(): | ||
| guided = sampler_module.GuidedDecoding() |
Copilot
AI
Nov 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The GuidedDecoding class constructor requires an fd_config parameter (with scheduler_config.max_num_seqs attribute), but this test creates it without any arguments. This will cause an AttributeError at runtime. You should pass a mock config object, similar to how it's done in other tests using fd_config_factory.
| def test_guided_decoding_tracks_processors_and_reasoning(): | |
| guided = sampler_module.GuidedDecoding() | |
| def test_guided_decoding_tracks_processors_and_reasoning(fd_config_factory): | |
| fd_config = fd_config_factory() | |
| guided = sampler_module.GuidedDecoding(fd_config) |
tests/model_executor/test_sampler.py
Outdated
| def test_sampler_compute_logprobs_handles_metadata(set_platform): | ||
| set_platform("cuda") | ||
| sampler = sampler_module.Sampler() |
Copilot
AI
Nov 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Creating a Sampler() without an fd_config parameter will cause an AttributeError because the Sampler.__init__ method calls GuidedDecoding(fd_config) which requires fd_config.scheduler_config.max_num_seqs to exist. You should pass a mock config object using fd_config_factory.
| def test_sampler_compute_logprobs_handles_metadata(set_platform): | |
| set_platform("cuda") | |
| sampler = sampler_module.Sampler() | |
| def test_sampler_compute_logprobs_handles_metadata(set_platform, fd_config_factory): | |
| set_platform("cuda") | |
| sampler = sampler_module.Sampler(fd_config_factory()) |
| def allocate_token_bitmask(self): | ||
| return paddle.zeros([2, 3], dtype="float32") |
Copilot
AI
Nov 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method requires 1 positional argument, whereas overridden LogitsProcessorBase.allocate_token_bitmask requires 3.
| def allocate_token_bitmask(self): | |
| return paddle.zeros([2, 3], dtype="float32") | |
| def allocate_token_bitmask(self, batch_size, num_tokens): | |
| return paddle.zeros([batch_size, num_tokens], dtype="float32") |
…sampler.py Fix sampler tests to pass config and guided decoding expectations
This commit adds unit tests for the sampler helpers, covering guided decoding and speculative flows. It includes various test cases to ensure the correct functionality of the sampling methods and processors.
Motivation
NO.20 功能模块 fastdeploy/model_executor/layers/sample/sampler.py 单测补充
Modifications
add tests/model_executor/test_sampler.py
Usage or Command
tests/model_executor/test_sampler.py:Accuracy Tests
tests/model_executor/test_sampler.py:Checklist
[FDConfig],[APIServer],[Engine],[Scheduler],[PD Disaggregation],[Executor],[Graph Optimization],[Speculative Decoding],[RL],[Models],[Quantization],[Loader],[OP],[KVCache],[DataProcessor],[BugFix],[Docs],[CI],[Optimization],[Feature],[Benchmark],[Others],[XPU],[HPU],[GCU],[DCU],[Iluvatar],[Metax]]pre-commitbefore commit.releasebranch, make sure the PR has been submitted to thedevelopbranch, then cherry-pick it to thereleasebranch with the[Cherry-Pick]PR tag.