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

Update segmentation_pipeline.py #30

Merged
merged 7 commits into from
Sep 7, 2023

Conversation

9527-csroad
Copy link
Contributor

GPU inference #27

Copy link
Collaborator

@Szuumii Szuumii left a comment

Choose a reason for hiding this comment

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

Good stuff! 🔥

Copy link
Owner

@wiktorlazarski wiktorlazarski left a comment

Choose a reason for hiding this comment

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

@9527-csroad Great first contribution 🚀! Thanks for all the changes. I'm leaving a one tiny request to be fixed rather easily.

Another thing I'd like to ask you (iff you have time) - would you consider updating the README.md file with model's time benchmarking you performed to solve that issue #27? I think that will be an interesting peace of information for future users/contributors.

Also can you update https://github.com/wiktorlazarski/head-segmentation/blob/main/head_segmentation/_version.py file to contain a new __version__ 1.3.0? Let's make release a new head_segmentation release once this PR is merged 🤗.

@@ -16,6 +16,7 @@ def __init__(
self,
model_path: str = C.HEAD_SEGMENTATION_MODEL_PATH,
model_url: str = C.HEAD_SEGMENTATION_MODEL_URL,
device = torch.device('cpu')
Copy link
Owner

Choose a reason for hiding this comment

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

Throughout the entire repo we tried to keep everything nicely typed. Let's keep on doing that even if type is explicitly tangible 😉.

Suggested change
device = torch.device('cpu')
device: torch.device = torch.device('cpu')

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for your wonderful project. I have updated the README.md file, and you can revise it as you want. This is my first time to PR, I'm not sure if there are any problems.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm sorry my PR is mess, in this process, I have learned some PR tricks and thanks for your project again!

Copy link
Owner

Choose a reason for hiding this comment

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

Sweet, no worries and please don't be sorry you did an amazing work 💪.

Choose a reason for hiding this comment

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

@9527-csroad thanks for update mate. I am also interested in this project. Would you mind sharing ur discord for a chat?

Copy link
Owner

@wiktorlazarski wiktorlazarski left a comment

Choose a reason for hiding this comment

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

Thanks for all updates and you contribution. Huge shoutout for updating the README.md file. I left one last minor comment/request but other than that everything else looks rock solid 🔥.

Congratulations on your first contribution to the project 🎉! If you want to contribute and learn through it 😉 more lemme know I have a few things in my mind that may be implemented in the repo to improve project but also don't hesitate to implement your ideas - that's always welcome 🤗.

Comment on lines 2 to 3
# __version__ = "1.1.0" # Segmentation model downloaded from GDrive
__version__ = "1.3.0" # add gpu inference
Copy link
Owner

Choose a reason for hiding this comment

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

I'd like to move changelog from the _version.py file to release notes. Can I ask you for a favour: would you remove those commented __version__'s tags. I'm looking for having a file which looks something like (contains only such single line as it's content):

Suggested change
# __version__ = "1.1.0" # Segmentation model downloaded from GDrive
__version__ = "1.3.0" # add gpu inference
__version__ = "1.3.0"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Happy to do something for this good project 🚀. If there are any problems, contact me and I will do it.

Copy link
Owner

Choose a reason for hiding this comment

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

Thanks @9527-csroad 🙌. Lemme collect my thoughts over a weekend and I'll spawn a bunch of issues that may improve the project.

I found the result that save by opencv will change the picture color.
@wiktorlazarski wiktorlazarski merged commit c1b4b9b into wiktorlazarski:main Sep 7, 2023
1 check passed
@9527-csroad 9527-csroad deleted the 9527-csroad-patch-1 branch September 8, 2023 01:04
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.

None yet

4 participants