-
Notifications
You must be signed in to change notification settings - Fork 315
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
support customize config path #423
support customize config path #423
Conversation
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.
run.py
Outdated
@@ -82,6 +82,11 @@ def parse_args(): | |||
'./outputs/default.', | |||
default=None, | |||
type=str) | |||
parser.add_argument('--custom-config-dir', |
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.
parser.add_argument('--custom-config-dir', | |
parser.add_argument('--config-dir', default='configs', |
In this way the parameter name can be shorter and more concise.
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.
done
opencompass/utils/run.py
Outdated
@@ -60,7 +60,8 @@ def get_config_from_arg(args) -> Config: | |||
raise ValueError('You must specify "--datasets" if you do not specify ' | |||
'a config file path.') | |||
datasets = [] | |||
for dataset in match_cfg_file('configs/datasets/', args.datasets): | |||
config_base_dir = args.custom_config_dir or 'configs' | |||
for dataset in match_cfg_file(f'{config_base_dir}/datasets/', args.datasets): |
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.
Using os.path.join
is better than string-style concatenation.
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.
done
c5f613f
to
82fe829
Compare
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.
LGTM
* support customize config path * support customize config path * support customize config path
If the config path can be customized, then when other projects introduce opencompass, they can maintain their own datasets/models configuration in their own projects:)