-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
[feat] Adding UPerNet #926
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
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.
Hi @brianhou0208, thanks a lot for working on this! Looks good to me, a few general notes/questions
- Can you please use more verbose variable naming, e.g.
ch->channels
,h -> height
(it is not hold over the repo, but would be nice to have it) - Please, add the model to the docs/ and README.md
- Did you have a chance to train it with any notebook example?
Hi @qubvel , I’ve updated the code according to the commit, along with the README and documentation. Due to device limitations, the default parameters caused an OOM error, so I adjusted the
|
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.
Thanks for update, I made a quick look at the architecture design, here is main concern
Resize all FPN output features to 1/4 of the original resolution.
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.
Looks great, just one nit and we are ready to merge this!
1. Use `SegmentationHead` for upsampling, set `upsampling=4` 2. Remove the additional variable `out_channels` from `UPerNetDecoder` 3. Fix `SegmentationHead` kernel size to 1
I've finished it and fixed a few small issues. |
@brianhou0208 Thanks a lot for addressing it! |
Description
I tried to add UperNet before. This may not be the best version. Maybe you can have a look.
@GrantorShadow @jawi289o
Issue
#906
Reference