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.

Sign up for GitHub

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

Jump to bottom

Adding a network CellSamWrapper #7981

Merged
merged 13 commits into from
Aug 10, 2024
Merged

Adding a network CellSamWrapper #7981

merged 13 commits into from
Aug 10, 2024

Conversation

myron
Copy link
Collaborator

@myron myron commented Aug 2, 2024

Adding a network CellSamWrapper, a thin wrapper around SAM, which can be used for 2D segmentation tasks.

Types of changes

  • Non-breaking change (fix or new feature that would not break existing functionality).
  • Breaking change (fix or new feature that would cause existing functionality to change).
  • New tests added to cover the changes.
  • Integration tests passed locally by running ./runtests.sh -f -u --net --coverage.
  • Quick tests passed locally by running ./runtests.sh --quick --unittests --disttests.
  • In-line docstrings updated.
  • Documentation updated, tested make html command in the docs/ folder.

am and others added 2 commits August 1, 2024 19:18
cell_sam_wrapper net
d7b910f 
Signed-off-by: am <am>
@pre-commit-ci
[pre-commit.ci] auto fixes from pre-commit.com hooks
51d63c9 
for more information, see https://pre-commit.ci

Signed-off-by: am <am>
KumoLiu
KumoLiu reviewed Aug 6, 2024
View reviewed changes
Copy link
Contributor

@KumoLiu KumoLiu 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 the PR, overall looks good me, several minor comments inline.

monai/networks/nets/cell_sam_wrapper.py Show resolved Hide resolved
monai/networks/nets/cell_sam_wrapper.py Show resolved Hide resolved
monai/networks/nets/cell_sam_wrapper.py Show resolved Hide resolved
tests/test_cell_sam_wrapper.py Outdated Show resolved Hide resolved
monai/networks/nets/cell_sam_wrapper.py Show resolved Hide resolved
@KumoLiu
Copy link
Contributor

KumoLiu commented Aug 7, 2024

Hi @myron, please help include the segment_anything in the requirements-dev.txt, then it will be installed in the ci env and test all the things. Thanks!

@myron
Copy link
Collaborator Author

myron commented Aug 9, 2024

Hi @myron, please help include the segment_anything in the requirements-dev.txt, then it will be installed in the ci env and test all the things. Thanks!

Hi, I can do it, but I don't think we should do it, it's a very limited use case, and I don't think we should ask every developer to install it.

@KumoLiu
Copy link
Contributor

KumoLiu commented Aug 9, 2024

Hi, I can do it, but I don't think we should do it, it's a very limited use case, and I don't think we should ask every developer to install it.

Hi @myron, yes we need to include that, then all the test cases can be tested correctly. Users don't need to install "requirement-dev", they can choose what they want to install like pip install monai["nibabel"].

myron added 2 commits August 9, 2024 10:36
@myron
Merge remote-tracking branch 'upstream/dev' into v2d
76f8244
@myron
edits
d7d624b 
Signed-off-by: myron <amyronenko@nvidia.com>
@myron
Copy link
Collaborator Author

myron commented Aug 9, 2024

Hi @myron, please help include the segment_anything in the requirements-dev.txt, then it will be installed in the ci env and test all the things. Thanks!

added

@myron
Copy link
Collaborator Author

myron commented Aug 9, 2024

all done, please check

pre-commit-ci bot and others added 2 commits August 9, 2024 17:51
@pre-commit-ci
[pre-commit.ci] auto fixes from pre-commit.com hooks
a41a615 
for more information, see https://pre-commit.ci
@KumoLiu
format fix
24afcb4 
Signed-off-by: YunLiu <55491388+KumoLiu@users.noreply.github.com>
KumoLiu
KumoLiu approved these changes Aug 10, 2024
View reviewed changes
Copy link
Contributor

@KumoLiu KumoLiu left a comment

Choose a reason for hiding this comment

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

@myron Thanks for the quick update. Just push a commit help fix format issue.

@KumoLiu
Copy link
Contributor

KumoLiu commented Aug 10, 2024

/build

@KumoLiu
fix format issue
8ee636f 
Signed-off-by: YunLiu <55491388+KumoLiu@users.noreply.github.com>
KumoLiu added 3 commits August 10, 2024 17:42
@KumoLiu
try add it in setup.cfg
b5c79f1 
Signed-off-by: YunLiu <55491388+KumoLiu@users.noreply.github.com>
@KumoLiu
add it in setup.cfg
6d4a047 
Signed-off-by: YunLiu <55491388+KumoLiu@users.noreply.github.com>
@KumoLiu
update installation md
53ac8e1 
Signed-off-by: YunLiu <55491388+KumoLiu@users.noreply.github.com>
@KumoLiu
Copy link
Contributor

KumoLiu commented Aug 10, 2024

/build

@KumoLiu KumoLiu merged commit 6243031 into Project-MONAI:dev Aug 10, 2024
28 checks passed
rcremese pushed a commit to rcremese/MONAI that referenced this pull request Sep 2, 2024
@myron @pre-commit-ci
@KumoLiu @rcremese
Adding a network CellSamWrapper (Project-MONAI#7981)
c29258c 
Adding a network CellSamWrapper, a thin wrapper around SAM, which can be
used for 2D segmentation tasks.



### Types of changes
<!--- Put an `x` in all the boxes that apply, and remove the not
applicable items -->
- [x] Non-breaking change (fix or new feature that would not break
existing functionality).
- [ ] Breaking change (fix or new feature that would cause existing
functionality to change).
- [ ] New tests added to cover the changes.
- [ ] Integration tests passed locally by running `./runtests.sh -f -u
--net --coverage`.
- [ ] Quick tests passed locally by running `./runtests.sh --quick
--unittests --disttests`.
- [ ] In-line docstrings updated.
- [ ] Documentation updated, tested `make html` command in the `docs/`
folder.

---------

Signed-off-by: am <am>
Signed-off-by: myron <amyronenko@nvidia.com>
Signed-off-by: YunLiu <55491388+KumoLiu@users.noreply.github.com>
Co-authored-by: am <am>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: YunLiu <55491388+KumoLiu@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KumoLiu KumoLiu KumoLiu approved these changes

@Nic-Ma Nic-Ma Nic-Ma left review comments

@ericspod ericspod ericspod left review comments

Assignees
No one assigned
Labels
None yet
Projects
No open projects
MONAI v1.4
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@myron @KumoLiu @ericspod @Nic-Ma