-
Notifications
You must be signed in to change notification settings - Fork 554
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
add blank_penalty for offline transducer #542
Conversation
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.
Thanks! Left some minor comments. Otherwise, it looks good to me.
const float *p_logit = logit.GetTensorData<float>(); | ||
float *p_logit = logit.GetTensorMutableData<float>(); | ||
if (blank_penalty_ > 0.0) { | ||
p_logit[0] -= blank_penalty_; // assuming blank id is 0 |
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.
Shall we also consider the case when batch_size > 1
?
p_logit[0]
is only for the first utterance.
We need to process
p_logit[vocab_size*i + 0]
// for in range(n)
You can move this if
statement into the for
statement below.
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.
Fixed!
@@ -862,6 +876,7 @@ def create_recognizer(args) -> sherpa_onnx.OfflineRecognizer: | |||
max_active_paths=args.max_active_paths, | |||
hotwords_file=args.hotwords_file, | |||
hotwords_score=args.hotwords_score, | |||
blank_penalty=args.blank_penalty, |
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.
Could you also update
def from_transducer( |
to add an extra argument
blank_penalty: float = 0
and also add docstring for it?
Note that you need to pass the argument to
recognizer_config = OfflineRecognizerConfig( |
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.
Fixed!
I also noticed there is no docstring for the hotwords params, as well as the max_active_paths was not propagated into the OfflineRecognizerConfig
. Should I raise two seperate PR to fix these?
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.
Should I raise two seperate PR to fix these?
Yes, please do that in a separate PR. Thanks!
By the way, please merge the master branch into your current branch so the CI can pass. |
Could you fix the c++ code style check?
You can run pip install clang-format
cd /path/to/sherpa-onnx
./scripts/check_style_cpplint.sh
./scripts/check_style_cpplint.sh 1
./scripts/check_style_cpplint.sh 2 to check the style locally. |
Thank you for your first-time contribution! |
Fix #541
Tested with
sherpa-onnx-offline
. High penalty will force out more words.