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.

Sign up for GitHub

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

Convert to absolute paths in wordrank #1503

Merged
merged 5 commits into from
Jul 25, 2017
Merged

Convert to absolute paths in wordrank #1503

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5802bdb
convert to absolute paths for every command
parulsethi Jul 24, 2017
53158e1
use sorted in directory structure test
parulsethi Jul 24, 2017
7c13de9
move join() to var definition
parulsethi Jul 24, 2017
7345023
made requested changes
parulsethi Jul 25, 2017
5bbe888
change gensim pin to develop in dockerfile
parulsethi Jul 25, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 20 additions & 22 deletions gensim/models/wrappers/wordrank.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
from six import string_types
from smart_open import smart_open
from shutil import copyfile, rmtree
from os.path import join


logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -87,28 +88,26 @@ def train(cls, wr_path, corpus_file, out_name, size=100, window=15, symmetric=1,
`ensemble` = 0 (default), use ensemble of word and context vectors
"""

meta_data_path = 'matrix.meta'
vocab_file = 'vocab.txt'
temp_vocab_file = 'tempvocab.txt'
cooccurrence_file = 'cooccurrence'
cooccurrence_shuf_file = 'wiki.toy'
meta_file = 'meta'

# prepare training data (cooccurrence matrix and vocab)
model_dir = os.path.join(wr_path, out_name)
meta_dir = os.path.join(model_dir, 'meta')
model_dir = join(wr_path, out_name)
Copy link
Owner

@piskvorky piskvorky Jul 25, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Using full namespace os.path.join is preferable.

There are many joins in Python and its various libraries, and the context makes the code immediately easier to read and understand for other readers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

meta_dir = join(model_dir, 'meta')
os.makedirs(meta_dir)
logger.info("Dumped data will be stored in '%s'", model_dir)
copyfile(corpus_file, os.path.join(meta_dir, corpus_file.split('/')[-1]))
os.chdir(meta_dir)
copyfile(corpus_file, join(meta_dir, corpus_file.split('/')[-1]))

vocab_file = join(meta_dir, 'vocab.txt')
temp_vocab_file = join(meta_dir, 'tempvocab.txt')
cooccurrence_file = join(meta_dir, 'cooccurrence')
cooccurrence_shuf_file = join(meta_dir, 'wiki.toy')
meta_file = join(meta_dir, 'meta')

cmd_vocab_count = ['../../glove/vocab_count', '-min-count', str(min_count), '-max-vocab', str(max_vocab_size)]
cmd_cooccurence_count = ['../../glove/cooccur', '-memory', str(memory), '-vocab-file', temp_vocab_file, '-window-size', str(window), '-symmetric', str(symmetric)]
cmd_shuffle_cooccurences = ['../../glove/shuffle', '-memory', str(memory)]
cmd_vocab_count = [join(wr_path, 'glove', 'vocab_count'), '-min-count', str(min_count), '-max-vocab', str(max_vocab_size)]
cmd_cooccurence_count = [join(wr_path, 'glove', 'cooccur'), '-memory', str(memory), '-vocab-file', temp_vocab_file, '-window-size', str(window), '-symmetric', str(symmetric)]
cmd_shuffle_cooccurences = [join(wr_path, 'glove', 'shuffle'), '-memory', str(memory)]
cmd_del_vocab_freq = ['cut', '-d', " ", '-f', '1', temp_vocab_file]

commands = [cmd_vocab_count, cmd_cooccurence_count, cmd_shuffle_cooccurences]
input_fnames = [corpus_file.split('/')[-1], corpus_file.split('/')[-1], cooccurrence_file]
input_fnames = [join(meta_dir, corpus_file.split('/')[-1]), join(meta_dir, corpus_file.split('/')[-1]), cooccurrence_file]
Copy link
Owner

@piskvorky piskvorky Jul 25, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

string.split('/') is not portable -- see os.path.split, os.path.basename etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

output_fnames = [temp_vocab_file, cooccurrence_file, cooccurrence_shuf_file]

logger.info("Prepare training data (%s) using glove code", ", ".join(input_fnames))
Expand All @@ -126,7 +125,7 @@ def train(cls, wr_path, corpus_file, out_name, size=100, window=15, symmetric=1,
with smart_open(cooccurrence_shuf_file, 'rb') as f:
numlines = sum(1 for line in f)
with smart_open(meta_file, 'wb') as f:
meta_info = "{0} {1}\n{2} {3}\n{4} {5}".format(numwords, numwords, numlines, cooccurrence_shuf_file, numwords, vocab_file)
meta_info = "{0} {1}\n{2} {3}\n{4} {5}".format(numwords, numwords, numlines, cooccurrence_shuf_file.split('/')[-1], numwords, vocab_file.split('/')[-1])
Copy link
Owner

Choose a reason for hiding this comment

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

Dtto on split.

Elsewhere in the file (and in gensim) the standard C-style %s %d %f string formatting is used; best to keep it consistent here as well.

Copy link
Contributor

@menshikh-iv menshikh-iv Jul 25, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

@piskvorky formatting with {}.format more preferable for Python now. I think we should use format method instead of C-style formatting.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

kept {}.format for now

f.write(meta_info.encode('utf-8'))

if iter % dump_period == 0:
Expand Down Expand Up @@ -158,8 +157,8 @@ def train(cls, wr_path, corpus_file, out_name, size=100, window=15, symmetric=1,

# run wordrank executable with wr_args
cmd = ['mpirun', '-np']
cmd.append(np)
cmd.append(os.path.join(wr_path, 'wordrank'))
cmd.append(str(np))
cmd.append(join(wr_path, 'wordrank'))
for option, value in wr_args.items():
cmd.append('--%s' % option)
cmd.append(str(value))
Expand All @@ -168,10 +167,9 @@ def train(cls, wr_path, corpus_file, out_name, size=100, window=15, symmetric=1,

# use embeddings from max. iteration's dump
max_iter_dump = iter - (iter % dump_period)
copyfile('model_word_%d.txt' % max_iter_dump, 'wordrank.words')
copyfile('model_context_%d.txt' % max_iter_dump, 'wordrank.contexts')
model = cls.load_wordrank_model('wordrank.words', os.path.join('meta', vocab_file), 'wordrank.contexts', sorted_vocab, ensemble)
os.chdir('../..')
os.rename('model_word_%d.txt' % max_iter_dump, join(model_dir, 'wordrank.words'))
os.rename('model_context_%d.txt' % max_iter_dump, join(model_dir, 'wordrank.contexts'))
model = cls.load_wordrank_model(join(model_dir, 'wordrank.words'), vocab_file, join(model_dir, 'wordrank.contexts'), sorted_vocab, ensemble)

if cleanup_files:
rmtree(model_dir)
Expand Down
6 changes: 3 additions & 3 deletions gensim/test/test_corpora.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -517,14 +517,14 @@ def test_non_trivial_structure(self):

corpus = textcorpus.TextDirectoryCorpus(dirpath)
filenames = list(corpus.iter_filepaths())
base_names = [name[len(dirpath) + 1:] for name in filenames]
expected = [
base_names = sorted([name[len(dirpath) + 1:] for name in filenames])
expected = sorted([
'0.txt',
'a_folder/1.txt',
'b_folder/2.txt',
'b_folder/3.txt',
'b_folder/c_folder/4.txt'
]
])
expected = [os.path.normpath(path) for path in expected]
self.assertEqual(expected, base_names)

Expand Down