Fix mlctl command line #26
Conversation
| @@ -1,5 +1,4 @@ | |||
| from random_words import RandomWords | |||
| RandomWords().get_random_words() | |||
| @@ -13,7 +12,7 @@ def __init__(self, job_type, project, name=None): | |||
| self.name = name | |||
| else: | |||
| # else make a new name randomly | |||
| word = RandomWords().random_word('d') | |||
| word = RandomWords().random_word() | |||
There was a problem hiding this comment.
Random word('d') was intentional because it picked words that start with d
There was a problem hiding this comment.
Understood
Not a big fan of using words, if we look at the implementation it loads the random words from a file and it is just a fixed list of words
https://github.com/tomislater/RandomWords/blob/master/random_words/nouns.dat
And the list of words are bigger or smaller depending on which character we choose. The list is largest when we don't enforce a starting character
Generally it is better to stick with random generator based on epoch, which is O(1) in time and memory and no extra disk access, and has much lower chance of collision, which I believe is the reason for going random in the first place
I will suggest we move to something cheaper and have lower chance of collision, and avoid potential bug for customer
There was a problem hiding this comment.
I agree with Clarence. Let's revisit before merging it to main.
| words = RandomWords().get_random_words() | ||
| self.name = f'mlctl-batch-{words[1]}' | ||
| word = RandomWords().random_word() | ||
| self.name = f'mlctl-batch-{word}' |
There was a problem hiding this comment.
Batch jobs aren't working yet FYI. It's on the to do list.
There was a problem hiding this comment.
Yup, just making it consistent while at it
mlctl Pull Request Template Guidelines
Goals of this PR 🎉
Fix mlctl command line that crashes
repo:
% mlctl
Traceback (most recent call last):
File "/Users/clarenceng/opt/miniconda3/envs/mls/bin/mlctl", line 33, in
sys.exit(load_entry_point('mlctl', 'console_scripts', 'mlctl')())
File "/Users/clarenceng/opt/miniconda3/envs/mls/bin/mlctl", line 25, in importlib_load_entry_point
return next(matches).load()
File "/Users/clarenceng/opt/miniconda3/envs/mls/lib/python3.7/site-packages/importlib_metadata/init.py", line 168, in load
module = import_module(match.group('module'))
File "/Users/clarenceng/opt/miniconda3/envs/mls/lib/python3.7/importlib/init.py", line 127, in import_module
return _bootstrap._gcd_import(name[level:], package, level)
File "", line 1006, in _gcd_import
File "", line 983, in _find_and_load
File "", line 967, in _find_and_load_unlocked
File "", line 677, in _load_unlocked
File "", line 728, in exec_module
File "", line 219, in _call_with_frames_removed
File "/Users/clarenceng/github/clarng/mlctl/mlctl/clis/cli.py", line 4, in
from mlctl.clis.train_cli import train
File "/Users/clarenceng/github/clarng/mlctl/mlctl/clis/train_cli.py", line 5, in
from mlctl.clis.common.utils import determine_infra_plugin_from_job, parse_train_yamls, docker_instructions
File "/Users/clarenceng/github/clarng/mlctl/mlctl/clis/common/utils.py", line 10, in
from mlctl.jobs.MlctlDeployJob import MlctlDeployJob
File "/Users/clarenceng/github/clarng/mlctl/mlctl/jobs/MlctlDeployJob.py", line 2, in
RandomWords().get_random_words()
AttributeError: 'RandomWords' object has no attribute 'get_random_words'
Fix usage of RandomWorlds and also made it consistent between steps
Also went and fix test_init that is currently not passing - dircmp runs filecmp under the hood
test_cli passes now except for one that is broken due to refactoring
Things to check on 🎯