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

Add the GrabCut example #193

Open
wants to merge 25 commits into
base: main
Choose a base branch
from
Open

Add the GrabCut example #193

wants to merge 25 commits into from

Conversation

maximlt
Copy link
Contributor

@maximlt maximlt commented Nov 15, 2021

This PR adds the GrabCut example taken from Earthsim's topics. While the original example relied on the earthsim library, this example relies on the module earthsim.py that sits next to the notebook. This module has been extracted and adapted from earthsim. Doing so has allowed to significantly reduce the number of dependencies required to run the grabcut example, and as a consequence made it possible to update the remaining ones.

Compared to the original example:

  • No more dependency on quest to download map tiles, instead the module relies on the function get_tile_rgb available in the util.py module of geoviews.
  • The last step that used a PolyAnnotator to modify the contour generated during the main/previous stage has been entirely removed since annotators are currently broken in geoviews (annotators wont work in geoviews  holoviz/geoviews#526).
  • Added a few comments and some metadata (authors, last updated...).

@maximlt maximlt mentioned this pull request Nov 15, 2021
2 tasks
@maximlt
Copy link
Contributor Author

maximlt commented Nov 15, 2021

@jlstevens if you don't mind having a look at this, I think that this time we're closer to what we wanted to have from the beginning! Too bad that the annotators seem to be broken in geoviews, I just realized it quite late in the process of updating the main grabcut class.

@jbednar
Copy link
Contributor

jbednar commented Nov 15, 2021

annotators seem to be broken in geoviews

That's alarming!!

@jlstevens
Copy link
Contributor

Thanks for the PR!

I'll try this out tonight - this will be a nice test case for my deployment tool. :-)

annotators seem to be broken in geoviews

That sounds like an issue to file on geoviews if you can submit a small, reproducible example...

@maximlt
Copy link
Contributor Author

maximlt commented Nov 15, 2021

@jlstevens thanks! And there was already an issue opened on geoviews 🙃

Copy link
Contributor

@jbednar jbednar left a comment

Choose a reason for hiding this comment

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

Nice!

grabcut/anaconda-project.yml Outdated Show resolved Hide resolved
grabcut/grabcut.ipynb Outdated Show resolved Hide resolved
grabcut/grabcut.ipynb Outdated Show resolved Hide resolved
grabcut/grabcut.ipynb Outdated Show resolved Hide resolved
grabcut/grabcut.ipynb Outdated Show resolved Hide resolved
grabcut/grabcut.py Outdated Show resolved Hide resolved
grabcut/grabcut.ipynb Outdated Show resolved Hide resolved
@jlstevens
Copy link
Contributor

I've rebased this PR and if I can get it to build I'll merge to master and update the public website.

@jlstevens
Copy link
Contributor

jlstevens commented Jan 27, 2022

@maximlt just reminded me that there is one last change to be made to the grabcut project: we need to serve a panel dashboard with .servable() if we want to deploy (with some necessary hackery due to the lack of openGL in the deployment containers).

@maximlt
Copy link
Contributor Author

maximlt commented Feb 21, 2022

@jlstevens I've made Grabcut servable by creating a small dashboard with a pipeline that includes the first two stages introduced in the notebook. I've not added the 3rd stage (allow to manually edit the contour obtained after running grabcut) since the 4th stage which should allow to download the output (or do something with it, anything really) isn't included in the notebook, since it wasn't included in Earthsim. I don't even know if it's possible to export a Path or Polygon to a shapefile with geoviews?

Last section of the notebook:
image

First stage of the deployed app:
image

@jlstevens
Copy link
Contributor

Looks great!

A two stage pipeline is fine for now: I'll see if I can deploy a running version of this project shortly and if that works well, we can think about whether it is worth introducing more stages.

@jbednar
Copy link
Contributor

jbednar commented Feb 22, 2022

Cool! I believe shapely can export a shapefile.

@maximlt
Copy link
Contributor Author

maximlt commented Feb 22, 2022

Cool! I believe shapely can export a shapefile.

That would be with fiona, I don't think shapely does any I/O. Maybe also with pyshp.

@ahuang11
Copy link
Contributor

ahuang11 commented Mar 1, 2024

Oh man, I can definitely understand why Maxime had concerns about panel-chat-examples and future maintenance; for old, unmaintained repos, its a nightmare.

Took me the whole day to get it barely functioning. It's cool when it works, but super, extremely finicky.

Area.mp4

This drained the life out of me, and I'd prefer to rebuild existing ideas thru slightly more modern tools like https://twitter.com/giswqs/status/1649416858645282822 instead next time.

@@ -320,8 +320,8 @@ def view(self):
dmap = hv.DynamicMap(self.extract_foreground)
dmap = hv.util.Dynamic(dmap, operation=self._filter_contours)
dmap = hv.util.Dynamic(dmap, operation=self._simplify_contours)
return (regrid(self.image).options(**options) * self.bg_paths * self.fg_paths +
Copy link
Contributor

@ahuang11 ahuang11 Mar 1, 2024

Choose a reason for hiding this comment

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

Why was the regrid add here? It kept making the shapes mismatched and took me forever to find what caused the shape mismatches :(

Copy link
Contributor

Choose a reason for hiding this comment

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

Presumably to make it possible to display really large images without causing issues in the browser? That would be my initial guess at least.

grabcut/grabcut.py Outdated Show resolved Hide resolved
@maximlt
Copy link
Contributor Author

maximlt commented Mar 1, 2024

Oh man, I can definitely understand why Maxime had concerns about panel-chat-examples and future maintenance; for old, unmaintained repos, its a nightmare.

Welcome to the gang :D Code that doesn't run dies quicker than one would think!

@ahuang11
Copy link
Contributor

Would like a review on this.

@maximlt maximlt mentioned this pull request Apr 21, 2024
25 tasks
Copy link
Contributor

@jbednar jbednar left a comment

Choose a reason for hiding this comment

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

Looks great to me! No longer any outlandish imports!

"\n",
"Written by Philipp Rudiger and Maxime Liquet<br>\n",
"Created: November 16, 2018<br>\n",
"Last updated: January 7, 2021"
Copy link
Contributor

Choose a reason for hiding this comment

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

Do the dates auto-update on merge?

Copy link
Contributor

Choose a reason for hiding this comment

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

No I don't think so.

grabcut/grabcut.ipynb Outdated Show resolved Hide resolved
@ahuang11
Copy link
Contributor

Can we merge this?

@jbednar
Copy link
Contributor

jbednar commented Apr 29, 2024

Please!

@maximlt
Copy link
Contributor Author

maximlt commented Apr 29, 2024

Let me review please, I see some changes to make, to the project config for instance...

@maximlt
Copy link
Contributor Author

maximlt commented Apr 30, 2024

@ahuang11 did you use anaconda-project to set up your environment to work on Grabcut? I see the project is not locked on osx-arm64.

@ahuang11
Copy link
Contributor

It's been a while so I don't really remember. I believe I did, but I didn't lock the project.

@maximlt
Copy link
Contributor Author

maximlt commented Dec 8, 2024

Looks great to me! No longer any outlandish imports!

Sorry to come back to this old message @jbednar, I'm sure @ahuang11 would agree that "great" isn't an adjective that qualifies GrabCut 🙃 I don't know if you ran the notebook but I just did, after relocking since the last lock didn't support osx-arm64, and, oh boy, that's a painful example to run through. I wrote down a list of bugs to resolve and suggestions to improve the UX, which is pretty terrible at the moment. It's a non-trivial amount of work and merging GrabCut is just a nice-to-have on my current Examples todo list (#383), so I won't prioritize it, unless told so.

  • It's very easy to get confused with mapping background/foreground to water/land (or the other way around!)
  • Display activity (e.g. loading indicator, notifications)
  • Fix the update contour algorithm that somehow seems to produce two contours instead of one?
    image
  • Does the PolyEdit tool still work? I've tried many, many, times to edit the contour(s) in the notebook and never managed to do so. Among many tries, I've managed to edit a polygon on the HoloViews reference page only once, enough to show me it somehow works, which annoyed me quite decently as I never managed to get it to work again (https://holoviews.org/reference/streams/bokeh/PolyEdit.html) EDIT: see Add the GrabCut example #193 (comment). How to use the PolyEdit tool isn't obvious, the docs should very clearly explain the process involved (possibly with a video/GIF, see the recent neuroscience examples added by Demetris for reference).
  • Since the update contour algorithm is broken, the paths to polys procedure produces bad results
    image
    image

versus (from https://earthsim.holoviz.org/topics/GrabCut.html)
image

  • On the final dashboard, I managed to pass the first stage, though creating the box only worked when I used SHIFT+drag (double click did nothing). I didn't manage to get through the second stage, I didn't try to debug it.

In my OP 3 years ago I wrote:

The last step that used a PolyAnnotator to modify the contour generated during the main/previous stage has been entirely removed since annotators are currently broken in geoviews (annotators wont work in geoviews holoviz/geoviews#526).

Copy link
Contributor

github-actions bot commented Dec 8, 2024

Your changes were successfully integrated in the dev site, make sure to review the pages of the projects you touched before merging this PR.

@jbednar
Copy link
Contributor

jbednar commented Dec 9, 2024

Looks great to me! No longer any outlandish imports!

Sorry to come back to this old message @jbednar, I'm sure @ahuang11 would agree that "great" isn't an adjective that qualifies GrabCut 🙃 I don't know if you ran the notebook but I just did, after relocking

What looked great to me (and still looks great) is that anaconda-project.yml no longer includes anything apparently crazy; those are all released, normal packages, and that's a great improvement. I don't know if I ran the code at the time, and I haven't run it today, but I agree that the behavior you outline above doesn't seem good. This example was one of the motivators for how we developed our ideas around editing and annotating shapes, so it's not surprising that it has a lot of barely ok ways of doing those things. I have very often wanted to demo the ideas covered by this example, but have been stymied by it being unable to build or having functionality that wasn't working. I'm perfectly happy with a different approach to showing the same thing, and don't need the algorithm involved to be grabcut specifically, though it's nice that it's simpler than e.g. Segment Anything. In any case the actual segmentation performance isn't the main point of the example, but instead it's about being able to draw shapes, then select regions by value based on the drawings, then annotate them, and then use that shape in subsequent computations. Those are all very important capabilities that we are currently almost entirely hiding from our users without having this example running.

@maximlt
Copy link
Contributor Author

maximlt commented Dec 9, 2024

Oh yes, I agree the concept overall is okay and this example is a good candidate for the gallery. It's just that the example needs to be fixed (and maybe some upstream libraries too) before we publish it. I'm a bit sad we can't, as all the work I did on this repo was originally motivated by this example, merging new examples 3 years ago was pretty brittle. I don't have the time to work on these fixes at the moment, maybe early next year. Or someone else could pick this up. I don't think working on another non-GrabCut example using the same functionality will be less work so let's fix this one.

Another suggestion to improve the example:

  • The main and most interesting code is in the grabcut.py module. It should instead be exposed directly in the notebook, there's no need for this module to exist (remember this example was extracted from earthsim.pyviz.org that was itself a package).

@maximlt
Copy link
Contributor Author

maximlt commented Dec 10, 2024

Does the PolyEdit tool still work? I've tried many, many, times to edit the contour(s) in the notebook and never managed to do so. Among many tries, I've managed to edit a polygon on the HoloViews reference page only once, enough to show me it somehow works, which annoyed me quite decently as I never managed to get it to work again (https://holoviews.org/reference/streams/bokeh/PolyEdit.html)

Ok, so we figured this out! Bokeh's behavior changed in version 3.4, to enter the edit mode with the PolyEditTool you no longer double-click but press tap (more than 300ms). This was changed in bokeh/bokeh#12831. I've opened an issue to track updating HoloViews' docs accordingly holoviz/holoviews#6474

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.

4 participants