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.

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

updated loading pretrained model #49

Merged
merged 5 commits into from
Aug 8, 2019
Merged

Conversation

ThisIsIsaac
Copy link
Contributor

properly raises error when the trained model path is incorrect, prints whether training is starting from scratch or from provided model path.

properly raises error when the trained model path is incorrect, prints whether training is starting from scratch or from provided model path.
Copy link
Owner

@Kaixhin Kaixhin left a comment

Choose a reason for hiding this comment

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

Looks good and sensible so thanks a lot - just requesting some minor changes.

agent.py Outdated Show resolved Hide resolved
agent.py Outdated Show resolved Hide resolved
agent.py Outdated
elif args.model and not os.path.isfile(args.model):
raise FileNotFoundError(args.model)

else:
Copy link
Owner

Choose a reason for hiding this comment

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

I would delete the else branch as this is the default and most common behaviour, so there should only be a need to signpost a trained model.

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

ThisIsIsaac and others added 3 commits August 7, 2019 21:56
Co-Authored-By: Kai Arulkumaran <Kaixhin@users.noreply.github.com>
Co-Authored-By: Kai Arulkumaran <Kaixhin@users.noreply.github.com>
@ThisIsIsaac
Copy link
Contributor Author

Applied your suggestions. Thanks for taking a look

@Kaixhin
Copy link
Owner

Kaixhin commented Aug 7, 2019

Ah sorry for not spotting earlier - we can now do if args.model as a separate conditional, and then if os.path.isfile(args.model) and else nested underneath so we don't have two calls to os.path.isfile.

@ThisIsIsaac
Copy link
Contributor Author

Np. Done :)

@Kaixhin Kaixhin merged commit e2c2622 into Kaixhin:master Aug 8, 2019
BerenMillidge pushed a commit to BerenMillidge/Rainbow that referenced this pull request Dec 20, 2019
updated loading pretrained model
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants