-
Notifications
You must be signed in to change notification settings - Fork 30
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.
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
Our first PR! #1
Conversation
@log.enter('load_dataset') | ||
def load_dataset(config): | ||
"""Preparing TextDataset from raw data.""" | ||
data_dir = os.path.join(config['data_dir'], config['data_name']) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unify all config['xxx']
syntax to config.xxx
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some hyperparameter search tools may require configs as a dictionary.
Correct me if I'm wrong.
parser.add_argument('--display_iter', type=int, default=100, help='Log state after every n epochs (default: %(default)s)') | ||
parser.add_argument('--data_workers', type=int, default=1, help='Use multi-cpu core for data pre-processing (default: %(default)s)') | ||
parser.add_argument('--eval', action='store_true', help='Only run evaluation on the test set (default: %(default)s)') | ||
parser.add_argument('--load_checkpoint', help='The checkpoint to warm-up with (default: %(default)s)') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
load_checkpoint
is more like an action to me. Rename to, for example checkpoint_path
, to match the description The checkpoint to warm-up with
?
This is the first pull request.