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

Semantic segmentation plugin #1

Merged
merged 3 commits into from
Jul 20, 2020
Merged

Semantic segmentation plugin #1

merged 3 commits into from
Jul 20, 2020

Conversation

joemarshall
Copy link

Added support for multiple models of semantic segmentation including
deeplabv3plus which should work better than the existing deeplabv3

Mainly I was just trying here to see how hard it was to bring a new model in, but this does work a little bit better than the plain deeplabv3 plugin.

Added support for multiple models of semantic segmentation including
deeplabv3plus which should work better than the existing deeplabv3
@valgur
Copy link
Owner

valgur commented Jun 14, 2020

Hi! This is excellent and exactly the kind of flexibility with model choices I had in mind when I changed the GIMP-ML architecture to be based on the Torch Hub format.

I will gladly merge this PR, but I'm not too eager about the implication of creating a separate fork of GIMP-ML just yet. I would prefer these changes to be part of the main repo, but this is up to @kritiksoman. I guess I will have to fork GIMP-ML for good at some point if he remains unresponsive.

The code for the plugin and the added hubconf.py look good. There are some minor issues and cosmetic adjustments I would prefer to be included before this PR can be merged, though:

  • The plugin script must have the executable bit set for GIMP to recognize it as a plugin. I assume you are developing on Windows and this just worked semi-accidentally? Anyway, please apply git update-index --chmod=+x plugins/semanticsegmentation.py to make sure it works on Unix platforms also.
  • Please set progress=True for load_state_dict_from_url() in the hubconf.py file. This is required for having the model download progress displayed in GIMP. Better yet if you make this a parameter of the model callables in hubconf.py.
  • I would name the plugin file semantic_segmentation.py and the model file DeepLabv3.py. You can remove the previous DeepLabv3 plugin and model.
  • Please follow PEP 8 for the code style. I say this mostly regarding the variable name style, where you should prefer snake_case instead of camelCase for Python code unless there is a good reason not to.
  • For consistent formatting applying an auto-formatter would be nice. Good options are PyCharm's built-in auto-formatting and black.

@joemarshall
Copy link
Author

Oops, I've been programming in 3 different languages recently so I'm not hot on case styles and the like! Will fix these things in a couple of days once I've got time after I've finished marking student work.

Next thoughts for things I want in gimp is this deblur: it's surprisingly simple and the results look great. I'll try to add that at some point soon.

https://github.com/csdwren/SelfDeblur/blob/master/selfdeblur_nonblind.py

@valgur
Copy link
Owner

valgur commented Jun 15, 2020

Next thoughts for things I want in gimp is this deblur: it's surprisingly simple and the results look great. I'll try to add that at some point soon.

Looking forward to your PR. 👍
You can apply the same logic I used for adding MiDaS to the monocular depth plugin to provide a choice between the existing DeblurGANv2 model and SelfDeblur: https://github.com/valgur/GIMP-ML/blob/master/plugins/monocular_depth.py

@joemarshall
Copy link
Author

Think I've fixed the code style etc. on this now, and added the progress callbacks

@joemarshall
Copy link
Author

Code and permissions fixed now, think it is good to merge, but I can't test on unix here?

@valgur valgur merged commit 6cbb3d8 into valgur:master Jul 20, 2020
@valgur
Copy link
Owner

valgur commented Jul 20, 2020

Looks good to me now. Thank you!

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

Successfully merging this pull request may close these issues.

2 participants