fix command line run for refinforce_learn_qnet in pl_examples #5414
fix command line run for refinforce_learn_qnet in pl_examples #5414Borda merged 7 commits intoLightning-AI:masterfrom
Conversation
Codecov Report
@@ Coverage Diff @@
## master #5414 +/- ##
======================================
Coverage 93% 93%
======================================
Files 135 135
Lines 10005 10005
======================================
Hits 9339 9339
Misses 666 666 |
|
@Borda Sure :) Unrelated to this pr- this example does not allow command line arguments for Trainer as most of the other examples do. Can only pass model specific args. If you think this is will be useful lmk, will submit a pr for it |
| np.random.seed(0) | ||
|
|
||
| parser = argparse.ArgumentParser() | ||
| parser = argparse.ArgumentParser(add_help=False) |
There was a problem hiding this comment.
Why did you set add_help=False ? It is always better to enable help.
There was a problem hiding this comment.
The code doesn't run with add_help=True ( #5382)
Since we supply our own help text for each argument, I think we can set this as False. With set to False, running python pl_examples\domain_templates\reinforce_learn_Qnet.py --help still displays the custom help options we provided.
There was a problem hiding this comment.
edit: here add_help=False is in the parent parser, but for the parser adding model specific args add_help=True by default. When it's set to True on both parsers, this conflicting option error occurs so one of them can be set to False
* fix wrong argument in argparse * remove wrong default arg in argparser * disable add help argparse
What does this PR do?
Unused default arguments in argparse were removed from refinforce_learn_qnet.py that was causing an error through command line. Previously, it was adding model specific args,
warm_start_sizeandmax_episode_reward, both of which the model doesn't take inputs to. I also passadd_helper=FalsetoArgumentParserbecause of the conflicting help options error (issue 5382).warm_start_sizeandwarm_start_stepsboth mean the same thing, but onlywarm_start_stepsis used in the model so we can removewarm_start_sizeargument. Same as in its implementation in pl_boltsFixes #5381, #5382 (duplicate)
Before submitting
PR review
Anyone in the community is free to review the PR once the tests have passed.
Before you start reviewing make sure you have read Review guidelines. In short, see the following bullet-list:
Did you have fun?
Make sure you had fun coding 🙃