wko27
commented
9 years ago Hi there!
Fantastic work!
Do you mind making a few updates though before we merge it? Please feel free to ask questions if you have any :)
src/main/java/com/medallia/word2vec/Word2VecModel.java
- * Assume word2vec bin file uses little-endian representation, while java
- * uses big endian.
- */
- public static Word2VecModel fromBinFile(File file)
- throws IOException {
- return fromBinFile(file, ByteOrder.LITTLE_ENDIAN);
- } +
- public static Word2VecModel fromBinFile(File file, ByteOrder byteOrder)
- throws IOException {
The javadoc for the first should say:
"Forwards to {@link #fromBinFile(...)} with the default ByteOrder.LITTLE_ENDIAN".
The javadoc for the second method should say:
"@return {@link Word2VecModel} created from the binary representation output by the open source C version of word2vec using the given byte order"
It would also be nice to give a URL to the C open source project, but don't worry about it for now since Google Code is closing down ...
- DataInput in = null;
- if (byteOrder == ByteOrder.BIG_ENDIAN) {
- in = new DataInputStream(fis);
- } else {
- in = new SwappedDataInputStream(fis);
- }
Please use a ternary statement
- String vocab = sb.toString();
- vocabs.add(vocab);
in-line the variable?
- float f = in.readFloat();
- vectors.add((double) f);
same here
- fis.close();
This should be done in a finally block of a try statement.
int index = firstLine.indexOf(' ');
After this call, it's usually nice to ensure that the index isn't -1. Something like:
Preconditions.checkState(index != -1, "Expected a space in the first line of file '%s': '%s'", file.getAbsolutePath(), firstLine);
src/test/java/com/medallia/word2vec/Word2VecBinReaderTest.java
Excellent unit test! However, a few things:
Let's add a tiny bit of class-level javadoc indicating that it tests converting the binary models into {@link Word2VecModel}s. Also consider adding an @see to the actual Word2VecModel.fromBinFile method.
@Test
- public void test()
This method's javadoc should say something like
"Tests that the Word2VecModels created from a binary and text representations are equivalent"
- URL url = this.getClass().getResource("/com/medallia/word2vec/tokensModel.bin");
- File binFile = new File(url.getFile());
- Word2VecModel binModel = Word2VecModel.fromBinFile(binFile);
- Searcher binSearcher = binModel.forSearch(); +
- url = this.getClass().getResource("/com/medallia/word2vec/tokensModel.txt");
If you notice in the util package, there's a Common.readResourceToStream. Let's create a Common.readResourceToFile and use it here.
- private void testVector(List
txtVector, List binVector, - String vocab) {
- assertEquals(txtVector.size(), binVector.size());
- for (int i = 0; i < txtVector.size(); i++) {
- double txtD = txtVector.get(i);
- double binD = binVector.get(i);
- assertEquals(txtD, binD, 0.0001);
- }
- }
The vocab argument isn't being used, so let's remove that. Also, let's rename testVector to assertVectorsEqual.
Cheers, Andrew
yfpeng
This function assumes that the bin file created by the C word2vec tool uses little-endian to write floats into the bin file. Each float occupies 4 bytes (32 bits) specified by the IEEE 754 standard. To read the word2vec model from a bin file, you can
Word2VecModel model = Word2VecModel.fromBinFile(file);
Alternatively, there is another function that also allows you to specify the bye order of the bin file.
Word2VecModel model = Word2VecModel.fromBinFile(file, byteOrder);