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

Updated hypothesis key generation to be the same as sherpa #226

Merged
merged 2 commits into from
Jul 28, 2023

Conversation

jingzhaoou
Copy link
Contributor

As suggested by the subject. This makes it easier to compare results between sherpa and sherpa-onnx.

@@ -60,10 +60,10 @@ struct Hypothesis {
std::string Key() const {
// TODO(fangjun): Use a hash function?
std::ostringstream os;
std::string sep = "-";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please change it to

std::string sep;

and leave the code in the for loop unchanged.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@csukuangfj I changed the code as suggested, which looks like the following

  std::string Key() const {
    // TODO(fangjun): Use a hash function?
    std::ostringstream os;
    std::string sep;
    for (auto i : ys) {
      os << i << sep;
      sep = "-";
    }
    return os.str();
  }

For 0, 0, 120, the key is "00-120-". I assume 0-0-120 is the correct join result. Let me know if I miss anything. Thanks.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please change

os << i << sep;

to

os << sep << i;

Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@csukuangfj Just changed as suggested. Seem working well. Thanks a lot!

Copy link
Collaborator

@csukuangfj csukuangfj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@csukuangfj csukuangfj merged commit daffdab into k2-fsa:master Jul 28, 2023
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants