Skip to content
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

Replace FbankOptions with FeatureConfig to support 'normalize_samples' option #546

Merged
merged 2 commits into from
Feb 28, 2024

Conversation

megazone87
Copy link
Contributor

By setting --normalize-samples=false, you can invoke the model trained with Kaldi-style features.

Note: sherpa-offline supports setting this option from the command line, while sherpa-online does not support it. This PR is to enable command line setting support for sherpa-online.

Note 2: There are no changes related to pybind, server/client, triton (maybe) submitted here. If you think that 'Replace FbankOptions with FeatureConfig' is worth it (for this requirement or in the future), I am willing to make the necessary changes.

@csukuangfj csukuangfj added the cpp label Feb 27, 2024
@csukuangfj
Copy link
Collaborator

Thank you for your first-time contribution!

Looks good to me.
Will merge it after the CI tests get passed.

@csukuangfj
Copy link
Collaborator

Could you build and test it locally?

The CI gives compilation errors:

[ 74%] Built target sherpa-online-websocket-client-microphone
/home/runner/work/sherpa/sherpa/sherpa/csrc/test-online-stream.cc: In member function ‘virtual void sherpa::OnlineStream_Test_Test::TestBody()’:
/home/runner/work/sherpa/sherpa/sherpa/csrc/test-online-stream.cc:35:40: error: no matching function for call to ‘sherpa::OnlineStream::OnlineStream(kaldifeat::FbankOptions&)’
   35 |   OnlineStream s(feat_config.fbank_opts);
      |                                        ^
In file included from /home/runner/work/sherpa/sherpa/sherpa/csrc/test-online-stream.cc:23:
/home/runner/work/sherpa/sherpa/sherpa/cpp_api/online-stream.h:61:12: note: candidate: ‘sherpa::OnlineStream::OnlineStream(const sherpa::FeatureConfig&, sherpa::ContextGraphPtr)’
   61 |   explicit OnlineStream(const FeatureConfig &feat_config,
      |            ^~~~~~~~~~~~
/home/runner/work/sherpa/sherpa/sherpa/cpp_api/online-stream.h:61:46: note:   no known conversion for argument 1 from ‘kaldifeat::FbankOptions’ to ‘const sherpa::FeatureConfig&’
   61 |   explicit OnlineStream(const FeatureConfig &feat_config,
      |                         ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~

Please see
https://github.com/k2-fsa/sherpa/actions/runs/8063891860/job/22026637645

@csukuangfj csukuangfj added cpp and removed cpp labels Feb 28, 2024
@megazone87
Copy link
Contributor Author

I have made the changes and tested them locally. Can you restart the actions tests?

@csukuangfj csukuangfj merged commit dd8c367 into k2-fsa:master Feb 28, 2024
6 checks passed
@csukuangfj
Copy link
Collaborator

Thanks!

@megazone87 megazone87 deleted the feat_config branch February 28, 2024 12:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants