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

Inappropriate implementation of Multi-head in IPT? (which only works on SR) #55

Open
hyunW3 opened this issue Aug 16, 2023 · 4 comments

Comments

@hyunW3
Copy link

hyunW3 commented Aug 16, 2023

Hello, I'm interested in your work and try some simple things based on your work.
While i'm inspecting your code, I found some problem. I want to know whether it is real problem or misunderstanding

The passed argument to model in test time is degraded image and idx_scale as shown below

sr = self.model(rain, idx_scale)

I thought "idx_scale" stands for task id, which denote "i" in $f_H = H^i(x)$ you mentioned in the paper 3.1 Head.

image

However, I found that "idx_scale" only varies depending on scale level in SR(Super Resolution), and "idx_scale" is set at 0 on denoising and deraining task.

Also, the number of heads is decided with "args.scale" list, not number of tasks.

self.head = nn.ModuleList([
nn.Sequential(
conv(args.n_colors, n_feats, kernel_size),
common.ResBlock(conv, n_feats, 5, act=act),
common.ResBlock(conv, n_feats, 5, act=act)
) for _ in args.scale
])

I want to make sure this is right to improve your code (or repo)
Thank you reading this issue and looking forward your reponse.

@JGyoung-UCAS
Copy link

Hello, I'm interested in your work and try some simple things based on your work. While i'm inspecting your code, I found some problem. I want to know whether it is real problem or misunderstanding

The passed argument to model in test time is degraded image and idx_scale as shown below

sr = self.model(rain, idx_scale)

I thought "idx_scale" stands for task id, which denote "i" in fH=Hi(x) you mentioned in the paper 3.1 Head.

image However, I found that "idx_scale" only varies depending on scale level in SR(Super Resolution), and "idx_scale" is set at 0 on denoising and deraining task.

Also, the number of heads is decided with "args.scale" list, not number of tasks.

self.head = nn.ModuleList([
nn.Sequential(
conv(args.n_colors, n_feats, kernel_size),
common.ResBlock(conv, n_feats, 5, act=act),
common.ResBlock(conv, n_feats, 5, act=act)
) for _ in args.scale
])

I want to make sure this is right to improve your code (or repo) Thank you reading this issue and looking forward your reponse.

I met with the same problem. Did you solve it?

@HantingChen
Copy link
Collaborator

Sorry for the unclear code. This implement is right since we only release the test code. Your implement is a right way to modify it into a training code.

@hyunW3
Copy link
Author

hyunW3 commented Oct 6, 2023

@JGyoung-UCAS Actually, i'm not working with this code right now..
However, sharing my thoughts with you,
I think the implementation of the "self.head" should be concerning with "task_id", not "args.scale" to separate the head for each task. This might requires separation of "args.scale" variable into "task" and "scale in SR task"

@HantingChen Thank you for your response,
My question is whether the multi-head is well implemented regardless of training or test, since "idx_scale 0" head covers two task (denoising & deraining) with your implementation.

@JGyoung-UCAS
Copy link

@JGyoung-UCAS Actually, i'm not working with this code right now.. However, sharing my thoughts with you, I think the implementation of the "self.head" should be concerning with "task_id", not "args.scale" to separate the head for each task. This might requires separation of "args.scale" variable into "task" and "scale in SR task"

@HantingChen Thank you for your response, My question is whether the multi-head is well implemented regardless of training or test, since "idx_scale 0" head covers two task (denoising & deraining) with your implementation.

Thank you for the reminder. This code is very incomplete. Maybe you can check this code in the version of Minspore: https://gitee.com/mindspore/models/tree/master/research/cv/IPT. It seems to be more clear.

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

No branches or pull requests

3 participants