From a9f106ea546961086fb74e7050310681262f9ab0 Mon Sep 17 00:00:00 2001 From: jahorton Date: Mon, 13 Jul 2020 08:53:22 +0700 Subject: [PATCH 1/4] refactor(developer): hashmap-based wordlist compile --- .../lexical-model-compiler/build-trie.ts | 19 ++----- developer/js/tests/test-parse-wordlist.ts | 49 +++++++++---------- 2 files changed, 29 insertions(+), 39 deletions(-) diff --git a/developer/js/source/lexical-model-compiler/build-trie.ts b/developer/js/source/lexical-model-compiler/build-trie.ts index 224450cfca..2a106208c0 100644 --- a/developer/js/source/lexical-model-compiler/build-trie.ts +++ b/developer/js/source/lexical-model-compiler/build-trie.ts @@ -4,7 +4,7 @@ import { readFileSync } from "fs"; * A word list is an array of pairs: the concrete word form itself, followed by * a non-negative count. */ -export type WordList = [string, number][]; +export type WordList = {[wordform: string]: number}; /** * Returns a data structure that can be loaded by the TrieModel. @@ -19,7 +19,7 @@ export function createTrieDataStructure(filenames: string[], searchTermToKey?: ( throw new TypeError("searchTermToKey must be explicitly specified") } // Make one big word list out of all of the filenames provided. - let wordlist: WordList = []; + let wordlist: WordList = {}; filenames.forEach(filename => parseWordListFromFilename(wordlist, filename)); let trie = Trie.buildTrie(wordlist, searchTermToKey as Trie.SearchTermToKey); @@ -101,17 +101,7 @@ export function parseWordList(wordlist: WordList, contents: string): void { count = 1; } - // TODO: this merge is very naive. We should consider whether the merge - // needs to be a little more aggressive. This may also be slow for large - // wordlists; probably O(n log n). We could improve this with a hash table - // if it becomes a performance problem. - const item = wordlist.find(value => value[0] === wordform); - if(item) { - item[1] += count; - } - else { - wordlist.push([wordform, count]); - } + wordlist[wordform] = (wordlist[wordform] || 0) + count; } } @@ -243,8 +233,9 @@ namespace Trie { * @param words a list of word and count pairs. */ buildFromWordList(words: WordList): Trie { - for (let [wordform, weight] of words) { + for (let wordform of Object.keys(words)) { let key = this.toKey(wordform); + let weight = words[wordform]; addUnsorted(this.root, { key, weight, content: wordform }, 0); } sortTrie(this.root); diff --git a/developer/js/tests/test-parse-wordlist.ts b/developer/js/tests/test-parse-wordlist.ts index e709aa5dda..48fa88d692 100644 --- a/developer/js/tests/test-parse-wordlist.ts +++ b/developer/js/tests/test-parse-wordlist.ts @@ -4,31 +4,31 @@ import 'mocha'; import { makePathToFixture } from './helpers'; const BOM = '\ufeff'; -const SENCOTEN_WORDLIST = [ - ['TŦE', 13644], - ['E', 9134], - ['SEN', 4816], - ['Ȼ', 3479], - ['SW̱', 2621], - ['NIȽ', 2314], - ['U¸', 2298], - ['I¸', 1988], - ['ȻSE', 1925], - ['I', 1884] -]; +const SENCOTEN_WORDLIST = { + 'TŦE': 13644, + 'E': 9134, + 'SEN': 4816, + 'Ȼ': 3479, + 'SW̱': 2621, + 'NIȽ': 2314, + 'U¸': 2298, + 'I¸': 1988, + 'ȻSE': 1925, + 'I': 1884 +}; describe('parseWordList', function () { it('should remove the UTF-8 byte order mark from files', function () { let word = 'hello'; let count = 1; - let expected = [ - [word, count] - ]; + let expected: WordList = {}; + expected[word] = count; + let file = `# this is a comment\n${word}\t${count}`; - let withoutBOM: WordList = []; + let withoutBOM: WordList = {}; parseWordList(withoutBOM, file); assert.deepEqual(withoutBOM, expected, "expected regular file to parse properly"); - let withBOM: WordList = []; + let withBOM: WordList = {}; parseWordList(withBOM, `${BOM}${file}`) assert.deepEqual(withBOM, expected, "expected BOM to be ignored"); }); @@ -36,7 +36,7 @@ describe('parseWordList', function () { it('should read word lists in UTF-8', function () { // N.B.: this is the format exported by Google Drive when selecting "TSV". const filename = makePathToFixture('example.qaa.sencoten', 'wordlist.tsv'); - let wordlist: WordList = []; + let wordlist: WordList = {}; parseWordListFromFilename(wordlist, filename); assert.deepEqual(wordlist, SENCOTEN_WORDLIST); }); @@ -45,7 +45,7 @@ describe('parseWordList', function () { // N.B.: this is the format exported by MS Excel when selecting // "UTF-16" text (tested on Excel for macOS). const filename = makePathToFixture('example.qaa.utf16le', 'wordlist.txt'); - let wordlist: WordList = []; + let wordlist: WordList = {}; parseWordListFromFilename(wordlist, filename); assert.deepEqual(wordlist, SENCOTEN_WORDLIST); }); @@ -53,7 +53,7 @@ describe('parseWordList', function () { it('should NOT read word lists in UTF-16 big-endian (with BOM)', function () { // N.B.: Does anything output this format...? const filename = makePathToFixture('example.qaa.utf16be', 'wordlist.txt'); - let wordlist: WordList = []; + let wordlist: WordList = {}; assert.throws(() => { parseWordListFromFilename(wordlist, filename); }, 'UTF-16BE is unsupported'); @@ -69,17 +69,16 @@ describe('parseWordList', function () { ' hello ', //4, expect to trim whitespace 'hello']; //5 - const expected = [ - [ 'hello', 10 /* 1+4+5 trimmed and identical */ ], - [ 'hell'+String.fromCharCode(0x00F3), 5 /* 2+3 normalised to NFC */ ] - ]; + const expected: WordList = {}; + expected['hello'] = 10; /* 1+4+5 trimmed and identical */ + expected['hell'+String.fromCharCode(0x00F3)] = 5 /* 2+3 normalised to NFC */ // Build a wordlist from the array let file = `# this is a comment\n`; for(let i = 0; i < words.length; i++) { file += `${words[i]}\t${i+1}\n`; } - let repeatedWords: WordList = []; + let repeatedWords: WordList = {}; parseWordList(repeatedWords, file); assert.deepEqual(repeatedWords, expected); From d74f8bef2d77826d6f6bbe11721d8f70d012a7e4 Mon Sep 17 00:00:00 2001 From: jahorton Date: Mon, 13 Jul 2020 11:46:15 +0700 Subject: [PATCH 2/4] docs(developer): tweaks WordList type documentation --- developer/js/source/lexical-model-compiler/build-trie.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/developer/js/source/lexical-model-compiler/build-trie.ts b/developer/js/source/lexical-model-compiler/build-trie.ts index 2a106208c0..ebd7e6a64a 100644 --- a/developer/js/source/lexical-model-compiler/build-trie.ts +++ b/developer/js/source/lexical-model-compiler/build-trie.ts @@ -1,8 +1,11 @@ import { readFileSync } from "fs"; /** - * A word list is an array of pairs: the concrete word form itself, followed by - * a non-negative count. + * A word list is (conceptually) an array of pairs: the concrete word form itself + a + * non-negative count. + * + * Since each word should only appear once within the list, we represent it with + * an associative array pattern keyed by the wordform. */ export type WordList = {[wordform: string]: number}; From c2346ffa1b80f9a58bdef6cdb2fddba48071aa25 Mon Sep 17 00:00:00 2001 From: Joshua Horton Date: Tue, 14 Jul 2020 07:51:26 +0700 Subject: [PATCH 3/4] refactor(developer): PR suggestion: Object.entries Co-authored-by: Eddie Antonio Santos --- developer/js/source/lexical-model-compiler/build-trie.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/developer/js/source/lexical-model-compiler/build-trie.ts b/developer/js/source/lexical-model-compiler/build-trie.ts index ebd7e6a64a..c804734511 100644 --- a/developer/js/source/lexical-model-compiler/build-trie.ts +++ b/developer/js/source/lexical-model-compiler/build-trie.ts @@ -236,9 +236,8 @@ namespace Trie { * @param words a list of word and count pairs. */ buildFromWordList(words: WordList): Trie { - for (let wordform of Object.keys(words)) { + for (let [wordform, weight] of Object.entries(words)) { let key = this.toKey(wordform); - let weight = words[wordform]; addUnsorted(this.root, { key, weight, content: wordform }, 0); } sortTrie(this.root); From 19d19891ba848c74d0220ca5aca9a61c131e6289 Mon Sep 17 00:00:00 2001 From: Joshua Horton Date: Tue, 14 Jul 2020 07:52:02 +0700 Subject: [PATCH 4/4] refactor(developer): PR suggestion: NFC literal Co-authored-by: Eddie Antonio Santos --- developer/js/tests/test-parse-wordlist.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/developer/js/tests/test-parse-wordlist.ts b/developer/js/tests/test-parse-wordlist.ts index 48fa88d692..3759c7c970 100644 --- a/developer/js/tests/test-parse-wordlist.ts +++ b/developer/js/tests/test-parse-wordlist.ts @@ -69,9 +69,10 @@ describe('parseWordList', function () { ' hello ', //4, expect to trim whitespace 'hello']; //5 - const expected: WordList = {}; - expected['hello'] = 10; /* 1+4+5 trimmed and identical */ - expected['hell'+String.fromCharCode(0x00F3)] = 5 /* 2+3 normalised to NFC */ + const expected: WordList = { + 'hello': 10, /* 1+4+5 trimmed and identical */ + 'hell\u00f3': 5, /* 2+3 normalised to NFC */ + }; // Build a wordlist from the array let file = `# this is a comment\n`; @@ -84,4 +85,3 @@ describe('parseWordList', function () { assert.deepEqual(repeatedWords, expected); }); }); -