Repository navigation
Conversation
a032a4d to
d80621f
Compare
Adds the three chat-completions sampling controls to /v1/chat/completions and wires them through every sampling path. - ServerAPITypes: parse frequency_penalty, presence_penalty, logit_bias (token-id-string -> bias map); validated in ChatHandler (penalties in [-2, 2], bias in [-100, 100], token ids bounds-checked; invalid input returns 400). - SamplingConfiguration + AdditivePenaltyProcessor: additive, count-scaled frequency penalty, once-per-token presence penalty, and per-token logit bias, applied on the CPU sampler path (sequential/static/VLM engines). Distinct from the existing multiplicative repetition penalty. - Pipelined GPU engine: a fused additive-delta stage in both the composite and argmax MPSGraph samplers (AdditivePenaltyGPUState), so the params take effect on the default engine. Greedy + penalties/bias is supported. The additive delta carries the same N-1 pipeline-delay handling as the repetition penalty. - Guided generation: penalties/bias applied before the grammar mask so token bans stay authoritative. - Fixes a latent cross-request issue: the pipelined engine's cached sampler is now invalidated when the penalty/bias configuration changes across requests that share a prefix (PenaltySignature). Unit tests cover the CPU processor, the GPU delta state, and request decoding.
d80621f to
9d679af
Compare
| self.pipelineDepth = pipelineDepth | ||
| self.frequencyPenalty = Float(frequencyPenalty) | ||
| self.presencePenalty = Float(presencePenalty) | ||
| self.windowSize = windowSize ?? 256 |
There was a problem hiding this comment.
With frequency_penalty set and no window, a 600-token generation for instance applies the penalty to a token used at step 10 when sampled on CPU (constrained decoding) but not on the pipelined GPU engine. The same request produces different output depending on the engine
Let's pick one default and apply it to both paths
There was a problem hiding this comment.
the actual divergence fix lives at the engine call sites
| self.logits = logitsRef | ||
| self.cachedSampler = nil | ||
| self.cachedSamplerTemperature = nil | ||
| self.cachedSamplerPenaltySignature = nil |
There was a problem hiding this comment.
should we initialize self.additivePenaltyState = nil here too?
| frequencyPenalty: Double, | ||
| presencePenalty: Double, |
There was a problem hiding this comment.
why are these Doubles when we immediately cast them to floats anyway?
There was a problem hiding this comment.
The external public API is Double
| self.frequencyPenalty = Float(frequencyPenalty) | ||
| self.presencePenalty = Float(presencePenalty) |
There was a problem hiding this comment.
Can make these optional and default to 0.0 to simplify API surface
| counts[tokenId, default: 0] += 1 | ||
| } | ||
| for (tokenId, count) in counts { | ||
| let idx = Int(tokenId) |
There was a problem hiding this comment.
feels like a lot of casts in this function. please see which ones we can remove
| for i in 0..<vocabSize { | ||
| ptr[i] = Float16(0.0) | ||
| } |
There was a problem hiding this comment.
double check this is the fastest way to do this, or that the impact is negligible
| /// | ||
| /// Valid range is [-2, 2]. Unlike `repetitionPenalty` (multiplicative, count-agnostic), | ||
| /// this is additive and scales with occurrence count. | ||
| public let frequencyPenalty: Double? |
There was a problem hiding this comment.
this gets cast to float In AdditivePenaltyGPUState, can we just make this float?
There was a problem hiding this comment.
All other fields are Double, if needed this should be a future refactor.
Co-authored-by: kevchengcodes <59463423+kevchengcodes@users.noreply.github.com>
- Fix cross-engine divergence (carinapeng): a nil repetitionPenaltyWindow means "all history" per the SamplingConfiguration contract. The CPU path honored it, but the GPU state silently capped at 256, so the same request could diverge by engine. Size the GPU ring to maxContextLength when the window is nil, for both the additive (frequency/presence) and repetition penalties. - Reset penaltyState / additivePenaltyState in the pipelined engine init for symmetry with the other cached-sampler fields. - Default AdditivePenaltyGPUState's frequency/presence/logitBias/window params so callers can omit them. - Replace the neutral additive buffer's per-element Float16 fill with a single memset (0.0 is all-zero bits).
Summary
Adds three chat-completions sampling controls:
frequency_penalty,presence_penalty, andlogit_biasto/v1/chat/completions, wired through every sampling path (CPU sequential/static/VLM engines, the pipelined GPU engine, and guided/constrained generation).Implementation
ServerAPITypes): three new optional fields, validated inChatHandler— penalties in[-2, 2],logit_biasvalues in[-100, 100], and token-id keys bounds-checked against the vocabulary; invalid input returns400.SamplingConfiguration, newAdditivePenaltyProcessor): additive, count-scaled frequency penalty; once-per-token presence penalty; and per-token logit bias. These are additive in logit space and distinct from the existing multiplicative repetition penalty.MPSGraphSamplers, newAdditivePenaltyGPUState): a fused additive-delta stage in both the composite and argmax samplers, so the options take effect on the default engine. Greedy decoding with penalties/bias is supported; the per-step delta carries the same N-1 pipeline-delay handling as the repetition penalty.PenaltySignature), preventing one request's options from leaking into the next.Enabling it
HTTP (chat-completions):
Swift (
SamplingConfiguration):Testing
logit_biasforcing, and cross-request non-contamination hold on every model, on both the sequential and pipelined engines. Two Phi variants (phi-3-mini, phi-3.5-mini) show a pre-existing base issue unrelated to this change.Notes
GenerationOptionsAPI does not expose per-token penalties or logit bias (only temperature, sampling mode, and max tokens), so they are offered through the server and the Swift sampler API.