Feature: Trainer without argparse #105
Conversation
tf2rl/experiments/trainer.py
Outdated
| args = policy.__class__.get_argument(Trainer.get_argument()) | ||
| args = args.parse_args([]) | ||
| for k, v in _args.items(): | ||
| setattr(args, k, v) |
There was a problem hiding this comment.
I guess this part overrides the arguments by specified dictionary.
Would it be possible to reject elements that are not defined in the args = policy.__class__.get_argument(Trainer.get_argument())?
There was a problem hiding this comment.
As long as argparse guarantees that there exist all the possible parameters including parameters not specified by command line, we can check with hasattr(args, k).
tests/experiments/test_trainer.py
Outdated
| batch_size=32, | ||
| n_warmup=10) | ||
| trainer = Trainer(policy, env, {}, test_env=test_env) | ||
| print(trainer) |
There was a problem hiding this comment.
To avoid linter error
There was a problem hiding this comment.
Could you tell me why this avoids linter error?
There was a problem hiding this comment.
https://github.com/ymd-h/tf2rl/runs/1047622916?check_suite_focus=true
F841 local variable 'trainer' is assigned to but never usedThere was a problem hiding this comment.
Maybe there are more better solutions, but checking whether parameters are default is also problematic.
These parameters can change their default values in future, which will break unit test.
There was a problem hiding this comment.
OK. I found a solution.
I simply do not assign to local value. (12cfa48)
This PR is related with #104
This PR enables
Trainerto acceptdict-typeargsat constructor.User can pass parameters like followings;
This PR preserves backward compatibility, so that user still can pass
argparse.Namespace(result ofparse_args()).