ngram hash function relies on compiler specific behavior

[leezu]

fastText uses std::string to represent text data, which is an alias for std::basic_string<char>. It is compiler defined if char is signed or unsigned. However, the result of

uint32_t Dictionary::hash(const std::string& str) const {
  uint32_t h = 2166136261;
  for (size_t i = 0; i < str.size(); i++) {
    h = h ^ uint32_t(str[i]);
    h = h * 16777619;
  }
  return h;
}

depends on if char is signed or unsigned.

To guarantee portability of binary fasttext models, std::basic_string<signed char> should be used throughout the library. This should match the current behavior on Linux and OS X (possibly more), but would change the behavior on Android systems for which char defaults apparently often to unsigned char. Ie. if anyone was using a pretrained binary fasttext model there it was giving wrong results.

[EdouardGrave]

Hi @leezu,

Thank you for reporting this issue.

You are right, and we will fix this issue by the end of the week.

Best,
Edouard.

[leezu]

Thanks @EdouardGrave for fixing. I'm closing the issue. For reference, the commit containing the fix is 9c9b9b2