-
Notifications
You must be signed in to change notification settings - Fork 223
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
[Feature] Add dynamic bricks #228
Conversation
Codecov Report
@@ Coverage Diff @@
## dev-1.x #228 +/- ##
==========================================
- Coverage 0.62% 0.55% -0.07%
==========================================
Files 125 130 +5
Lines 4641 5181 +540
Branches 719 810 +91
==========================================
Hits 29 29
- Misses 4607 5147 +540
Partials 5 5
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
|
||
from torch import nn | ||
|
||
from mmrazor.models.mutables.mutable_channel import MutableChannel | ||
from mmrazor.models.mutables.base_mutable import BaseMutable | ||
|
||
|
||
class DynamicOP(ABC): |
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.
suggestion:
add an interface, called convert_from(module). It is used to convert a basic torch module to a dynamic op.
raise RuntimeError(f'current choice of mutable {type(mutable)} ' | ||
'can not be None at runtime') | ||
|
||
return current_choice | ||
|
||
|
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.
Suggestion:
add unified interfaces, mutate_in_channels and mutate_out_channels, to ChannelDynamicOP, rather than using different methods to register mutables in different dynamic ops.
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.
Preferably in each Dynamic OP, there may be mutate_kernel_size
in Conv
, mutate_num_head
in MultiHeadAttention
.
The variable of each module is xx, and the corresponding api is mutate_xx
.
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.
My opinion is not only leaving two apis mutate_in_channels and mutate_out_channels, just using the same api name to register mutable channel, rather than 'mutate_in_channels' in dyanmicconv and 'mutate_num_features' in dynamicbn.
@@ -83,7 +83,9 @@ def num_choices(self) -> int: | |||
|
|||
def convert_choice_to_mask(self, choice: int) -> torch.Tensor: |
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.
suggestion: let MutableChannel only deal with mask, and implement the process which converts int/float/choice to mask, in another Mutable
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.
I have some suggestions, and the detail can be found in comments
|
||
|
||
@CONV_LAYERS.register_module() | ||
class DynamicConv2d(nn.Conv2d, ChannelDynamicOP): |
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.
expand DynamicConv2d
to DynamicConvXd
by inheriting from _ConvNd
* add dynamic bricks * add dynamic conv2d test * add tests for dynamic linear and dynamic norm * add docstring for dynamic conv2d * add docstring for dynamic linear * add docstring for dynamic batchnorm * Refactor the dynamic op ( put more logic into the mixin ) * fix UT * Fix UT ( fileio was moved to mmengine) * derived mutable adds choices property * Unify the register interface of mutable in dynamic op * Unified getter interface of mutable in dynamic op Co-authored-by: gaojianfei <gaojianfei@sensetime.com> Co-authored-by: pppppM <gjf_mail@126.com>
* mmcls fix ut * force add data folder * remove debug info * fix ut bugs
* check in the 2nd tutorial * update index * add new line * trailing whitespace * describe how to get srcnn.pth and face.png * rename file name * update index
Motivation
Add dynamic bricks
Modification
BC-breaking (Optional)
No.