Adding in NewContextWithStrategy for go binding #2948
Conversation
Adding in NewContextWithStrategy, a function to allow you to specify the strategy when initializing the context. NewContext will use this "helper" function so that users don't need to think about what strategy they're specifying by default. I'm also doing it this way (instead of a function that sets the strategy) because the basis of what parameters we populate the context with is determined by the strategy we are using, so it needs to be done in the beginning of creating the context.
|
I'm not sure if you want this, but I need to swap what strategy we use easily and figured it was useful, so I'd push it up and let you decide. |
danbev
left a comment
danbev
left a comment
There was a problem hiding this comment.
I'm not sure about these changes but perhaps we need to make the API more flexible. One thing I noticed is that Whisper_init does not take a whisper.Params which the java binding does. Perhaps that would be an option here as well.
| return model.NewContextWithStrategy(SAMPLING_GREEDY) | ||
| } | ||
|
|
||
| func (model *model) NewContextWithStrategy(strategy SamplingStrategy) (Context, error) { |
There was a problem hiding this comment.
Perhaps instead of passing in just the strategy to this function, it could accepts a parameter of type whisper.Params?
| // Make sure model adheres to the interface | ||
| var _ Model = (*model)(nil) | ||
|
|
||
| type SamplingStrategy whisper.SamplingStrategy |
There was a problem hiding this comment.
I think having this and the globals might be a little confusing.
There was a problem hiding this comment.
Yeah, the other option was to have clients import both versions, but figured that'd be more of a headache. Alright, no worries!
|
Closing this for now as I'm not sure about adding this at the moment, but feel free to re-open. |
3 participants
Adding in NewContextWithStrategy, a function to allow you to specify the strategy when initializing the context. NewContext will use this "helper" function so that users don't need to think about what strategy they're specifying by default. I'm also doing it this way (instead of a function that sets the strategy) because the basis of what parameters we populate the context with is determined by the strategy we are using, so it needs to be done in the beginning of creating the context.