Update spec algorithms to include samplingMode - #218
michaelwasserman merged 3 commits into
Conversation
Follow-up to webmachinelearning#206, webmachinelearning#210, and webmachinelearning#215. Those PRs updated the explainer and added the LanguageModelSamplingMode WebIDL declarations, but left out the algorithm steps in index.bs. - Make LanguageModel.samplingMode nullable (LanguageModelSamplingMode?) in the IDL to match LanguageModelCreateCoreOptions.samplingMode and Chromium's behavior when samplingMode is omitted. - Throw TypeError in validate and canonicalize language model options when both samplingMode and topK/temperature are provided. - Wire sampling mode into LanguageModel's internal slots, getter, creation, prefill, generate, and clone steps alongside top K and temperature. Bug: 561753033
|
cc: @tomayac |
| :: |options|["{{LanguageModelCreateCoreOptions/temperature}}"] if it [=map/exists=]; otherwise an [=implementation-defined=] value | ||
|
|
||
| : [=LanguageModel/sampling mode=] | ||
| :: |options|["{{LanguageModelCreateCoreOptions/samplingMode}}"] if it [=map/exists=]; otherwise null |
There was a problem hiding this comment.
Why not default to "balanced" so this attribute doesn't have to be nullable?
There was a problem hiding this comment.
We can do this, but will probably need to update the readme and our implementation (language_model.idl, added in crrev.com/c/7801217), because session.samplingMode is currently nullable (LanguageModelSamplingMode?) and returns null when samplingMode is omitted in create(). This is because omitting samplingMode falls back to the model's default_sampling_params (or legacy topK/temperature in extensions) rather than "balanced" (see ai_manager.cc). I think this is also why Mike removed the default in #215 instead of setting = "balanced".
@reillyeon @michaelwasserman Should we default it to "balanced" in create a language model object (and update Chromium to use "balanced" by default) or keep as is?
There was a problem hiding this comment.
It would be weird if leaving samplingMode unset resulted in some configuration that wasn't one of the defined sampling modes.
However, since we support custom sampling parameters in extensions we probably need to have some kind of "custom" value which is what you get when you don't pass a sampling mode but do pass temperature or topK. This would only be available to extensions.
There was a problem hiding this comment.
Sounds good, I will update create a language model object so sampling mode defaults to "balanced" when samplingMode is omitted (and follow up in Chromium to match).
For the extension case where deprecated topK or temperature are provided, I set it to return null (LanguageModelSamplingMode?) rather than adding "custom" to LanguageModelSamplingMode, since LanguageModelSamplingMode is also used as the input enum in LanguageModelCreateCoreOptions (and I think WebIDL cannot restrict a single enum value to extensions). That way standard sessions always default to "balanced", and null only occurs when legacy parameters are used.
There was a problem hiding this comment.
Done. I have also updated README.md slightly.
tomayac
left a comment
There was a problem hiding this comment.
LGTM from my side as well, and yes, as @reillyeon said, going with 'balanced' is the right default.
| :: |options|["{{LanguageModelCreateCoreOptions/temperature}}"] if it [=map/exists=]; otherwise an [=implementation-defined=] value | ||
|
|
||
| : [=LanguageModel/sampling mode=] | ||
| :: |options|["{{LanguageModelCreateCoreOptions/samplingMode}}"] if it [=map/exists=]; otherwise null if |options|["{{LanguageModelCreateCoreOptions/topK}}"] [=map/exists=] or |options|["{{LanguageModelCreateCoreOptions/temperature}}"] [=map/exists=]; otherwise "{{LanguageModelSamplingMode/balanced}}" |
There was a problem hiding this comment.
Given that there may be implementation-specific differences in the performance of different modes (e.g. most-predicable is the fastest) I considered some alternatives here:
- let the impl decide and just yield the result in the session interface attribute (not predictable for clients)
- pick
most-predictableas the default when unspecified (may not be the fastest in all impls)
Ultimately I think this is probably the right direction (balanced is the default), and we should consider adding a fastest mode that lets the implementation resolve any mode that is likely to yield the best performance.
SHA: 9fcb9a4 Reason: push, by michaelwasserman Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Update the Prompt API spec (
index.bs) with the algorithm definitions forsamplingMode. This is a follow-up to #206, #210, and #215, which updated the explainer (README.md) and WebIDL declarations, but did not yet update the spec algorithms.See issue #203.