jantic / DeOldify

A Deep Learning based project for colorizing and restoring old images (and video!)
MIT License
17.98k stars 2.56k forks source link

Code refactor and cleanup #140

Closed alexandrevicenzi closed 4 years ago

alexandrevicenzi commented 5 years ago

I noticed that many places could make use of some code cleanup and small refactor. This usually does not affect anything but makes the code more readable.

What I noticed:

What I propose:

If this is a good idea I'll start sending patches.

jantic commented 5 years ago

I'm a fan of your work so far so please, go ahead and do your thing!

jqueguiner commented 5 years ago

Thé split of API is due to overload of gpu. When having the 2 APIs in the same place the load into GPU being done at the main is allocating a lot of gpu memory which can result in lack of memory of you are ruining this on a small gpu.

jqueguiner commented 5 years ago

That being said I am ok to reverse this stuff

alexandrevicenzi commented 5 years ago

@jantic @jqueguiner what if we get rid of everything that is not DeOldify (code + notebooks) itself from this repo?

The idea would be making DeOldify a library, pip installable. For example pip install DeOldify. Everything else, API, apps, go to a spare repo, maybe DeOldifyApp, or any other name.

I believe that this would make more sense as @jantic is not interested in this part.

Also, thinking this way, what if someone wants an Android app, would it fit this repo? Of course not. So, the API might not be in this repo as well.

Making DeOldify a true python lib open new possibilities for a lot of extensions, like APIs, command-line tool, maybe even a desktop app.

jqueguiner commented 5 years ago

Personally I really love the idea but it’s up to Jason to decide !

jantic commented 5 years ago

@alexandrevicenzi @jqueguiner I'm totally in support of this. I personally can't devote much if anytime to this sort of overhaul at this point so this would be something I'd have to trust you with. It'd make sense to grant collaborator status if you're doing something of this magnitude. Just let me know if you want that. I'll still want to do code review and etc but really- I'm prepared to give you the keys and let you do what you think is right.

alexandrevicenzi commented 5 years ago

@jantic about the library I can do that, I have some projects on PyPI and I do similar work on my job as well.

Collaborator access would be great, I would never merge to master directly but you can force master protection on GitHub, so no one will merge to master, unless they are admins.

You may ask why I'm doing this, the main reason now is because I don't have a suitable graphics card to train the network and play with, but I really like this project.

jantic commented 5 years ago

@alexandrevicenzi Great idea on the master protection. I've put that in place. Collaborator invite is sent!

alexandrevicenzi commented 5 years ago

@jantic Thanks.

jantic commented 4 years ago

Going to close this as work, was done on this already.