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

Segment Anything 2 with 3D support #7965

Merged
merged 38 commits into from
Aug 13, 2024
Merged

Segment Anything 2 with 3D support #7965

merged 38 commits into from
Aug 13, 2024

Conversation

fm3
Copy link
Member

@fm3 fm3 commented Jul 31, 2024

URL of deployed dev instance (used for testing):

  • https://___.webknossos.xyz

Steps to test:

  • ssh -L 8080:localhost:8080 euphorix
  • use quick select in multiple viewports and mags and with differing depths

TODOs:

  • fix rgb support?
    • adapt max_request_size in torchserve
  • restore application.conf
  • restore feature snapshot test
  • remove the test-nux

Issues:


(Please delete unneeded items, merge only when none are left open)

Copy link
Contributor

@MichaelBuessemeyer MichaelBuessemeyer left a comment

Choose a reason for hiding this comment

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

Backend looks good to me so far :)

app/controllers/DatasetController.scala Show resolved Hide resolved
app/models/dataset/WKRemoteSegmentAnythingClient.scala Outdated Show resolved Hide resolved
@philippotto
Copy link
Member

@MichaelBuessemeyer I think you can already have a look at the frontend :)

@philippotto philippotto marked this pull request as ready for review August 9, 2024 08:16
@philippotto philippotto changed the title WIP: Segment Anything 2 Segment Anything 2 Aug 9, 2024
@philippotto philippotto changed the title Segment Anything 2 Segment Anything 2 with 3D support Aug 9, 2024
@MichaelBuessemeyer MichaelBuessemeyer self-requested a review August 9, 2024 08:46
Copy link
Contributor

@MichaelBuessemeyer MichaelBuessemeyer left a comment

Choose a reason for hiding this comment

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

Thanks a lot for all the effort to integrate SAM2. Works perfectly 🚀

During reviewing I only found minor suggestions and one confusion: One request param in the backend is annotated as "in target mag" and the frontend always seems to the bbox in mag 1.
But the feature still seems to work perfectly 🚀 -> Testing went well, I have no suggestions in that regard

conf/application.conf Outdated Show resolved Hide resolved
app/controllers/DatasetController.scala Outdated Show resolved Hide resolved
frontend/javascripts/admin/admin_rest_api.ts Show resolved Hide resolved
frontend/javascripts/libs/find_bounding_box_in_nd.ts Outdated Show resolved Hide resolved
frontend/javascripts/libs/find_bounding_box_in_nd.ts Outdated Show resolved Hide resolved
frontend/javascripts/oxalis/store.ts Show resolved Hide resolved
frontend/javascripts/navbar.tsx Show resolved Hide resolved
Copy link
Contributor

@MichaelBuessemeyer MichaelBuessemeyer left a comment

Choose a reason for hiding this comment

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

Frontend & Backend LGTM 🚀

Didn't do another round of testing as there weren't much functional change. Just ping me in case I should restest :)

@philippotto philippotto merged commit 31f18f2 into master Aug 13, 2024
2 checks passed
@philippotto philippotto deleted the sam2 branch August 13, 2024 13:35
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