-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Refactor loader_cfg. #214
Refactor loader_cfg. #214
Conversation
Codecov Report
@@ Coverage Diff @@
## master #214 +/- ##
=======================================
Coverage 82.27% 82.27%
=======================================
Files 145 145
Lines 6732 6732
Branches 1004 1004
=======================================
Hits 5539 5539
Misses 1086 1086
Partials 107 107
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
Thanks! I will check shortly |
Yeah thanks for the findings. It indeed causes error. I got another fancy way of merging dicts, do you want to check it out? loader_cfg = {
**dict((k, cfg.data[k]) for k in ['workers_per_gpu'] if k in cfg.data),
**dict(samples_per_gpu=1,
drop_last=False,
shuffle=False,
dist=distributed),
**cfg.data.get('train_dataloader', {})} |
Thanks, there are other places need to be modified: train.py L104, L170 and L278. |
pls install pre-commit and let it pass the lint :) |
mmedit/apis/train.py
Outdated
drop_last=False, | ||
dist=True, | ||
), | ||
**dict({} if torch.__version__ != 'parrots' else dict( |
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.
No there is no need to add this dict
Tests are failing |
mmedit/apis/train.py
Outdated
shuffle=False, | ||
drop_last=False, | ||
val_loader_cfg = { | ||
**dict(loader_cfg, shuffle=False, drop_last=False), |
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.
loader_cfg is a dict. To better demostrate the priority, put it outside
**loader_cfg,
**dict(shuffle=False, drop_last=False),
Thanks a lot! |
* Refactor loader_cfg. * Refactor loader_cfg in main() * Refactor 'loader_cfg', 'val_loader_cfg', and 'loader_cfg'. * Refactor 'loader_cfg's and 'val_loader_cfg'. * Refactor loader_cfg * Refactor loader_cfg and format code. * Refactor val_loader_cfg. Co-authored-by: root <root@cn0014004493l.domain.sensetime.com> Co-authored-by: 李尹硕 <SENSETIME\liyinshuo@cn0014004493l.domain.sensetime.com>
Refactor loader_cfg of main() in tools/test.py.
There is a bug in the master branch:
TypeError: type object got multiple values for keyword argument 'samples_per_gpu'
.That is because
loader_cfg
contains two'samples_per_gpu'
:samples_per_gpu=1,
**cfg.data.get('test_dataloader', {}),
, whilecfg.data['test_dataloader']=dict(samples_per_gpu=1),
The second one has been replaced by
loader_cfg.update(cfg.data.get('test_dataloader', {}))
.