Skip to content
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

mobilevit model v0.01 #120

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

XiaoheYou
Copy link
Collaborator

No description provided.

@wenmengzhou wenmengzhou added the enhancement New feature or request label Jul 12, 2022

"""

def __init__(self,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这种layer为什么不直接用nn.Dropout? 同其他layer

self.logger = get_root_logger()

@classmethod
def add_arguments(cls, parser: argparse.ArgumentParser):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

为什么需要这个接口? 移除

This class defines the MobileViTv2 architecture
"""

def __init__(self, *args, **kwargs) -> None:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

请参考其他backbone把主要参数在__init__函数中声明

self.reset_parameters(opts=opts)

@classmethod
def add_arguments(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

移除

print('-' * 67)
return input, overall_params, overall_macs

def profile_model(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这些不需要

return nn.Sequential(*block), input_channel


def MobileViTv2_postprocess(out):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个可以抽象到forward_test接口里,放到主干网络里

@@ -0,0 +1 @@
from .mobilevit_v2 import MobileViTv2, MobileViTv2_postprocess
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mobilevit为什么要单独开个目录,这个不符合之前的代码结构风格。beckbone,heads等抽象需要放到对应的目录里

@@ -0,0 +1,82 @@
#
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个layers目录是干啥的?看起来就是把官方代码抄了一份过来?这个完全没有按照EasyCV本身的设计理念进行开发。这个大的代码结构的调整,必须有详细的设计文档,以及调整的理由



__all__ = [
'GELU',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

relu,sigmoid等等,原util文件夹下本来就有,activation乃至layers文件夹存在的意义是什么?

return parser


__all__ = [
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这些norm为啥还要单独实现

Copy link
Collaborator

@jhuang1207 jhuang1207 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

看起来代码提交者对easyCV的整体设计风格没有足够的认识,建议
1 了解EasyCV代码结构并彻底重构mobilevit的实现
2 增加benchmark,提供复现精度和速度的文档
3 增加测试代码

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants