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

Adding mechanism to curate each extracted patch #653

Merged
merged 65 commits into from
Jul 6, 2023

Conversation

shubhaminnani
Copy link
Collaborator

@shubhaminnani shubhaminnani commented May 19, 2023

Added to code to curate the patches, discard the patches if tissue in patch is less than 30%, and if there is any other artifact as glass reflection, pen markings.

Fixes #652

Proposed Changes

  • added the code to curate the tiles after patching

Checklist

  • I have read the CONTRIBUTING guide.
  • My PR is based from the current GaNDLF master .
  • Non-breaking change (does not break existing functionality): provide as many details as possible for any breaking change.
  • Function/class source code documentation added/updated.
  • Code has been blacked for style consistency.
  • If applicable, version information has been updated in GANDLF/version.py.
  • If adding a git submodule, add to list of exceptions for black styling in pyproject.toml file.
  • Usage documentation has been updated, if appropriate.
  • History has been updated, if appropriate.
  • Tests added or modified to cover the changes; if coverage is reduced, please give explanation.
  • If customized dependency installation is required (i.e., a separate pip install step is needed for PR to be functional), please ensure it is reflected in all the files that control the CI, namely: python-test.yml, and all docker files [1,2,3].

Added to code to curate the patches, discard the patches if tissue in patch is less than 30%, and if there is any other artifact as glass reflection, pen markings.
@github-actions
Copy link
Contributor

github-actions bot commented May 19, 2023

MLCommons CLA bot All contributors have signed the MLCommons CLA ✍️ ✅

Copy link
Collaborator

@sarthakpati sarthakpati 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 your contribution!

Minor syntactic changes requested.

GANDLF/data/patch_miner/opm/patch_manager.py Outdated Show resolved Hide resolved
GANDLF/data/patch_miner/opm/patch_manager.py Outdated Show resolved Hide resolved
GANDLF/data/patch_miner/opm/patch_manager.py Outdated Show resolved Hide resolved
GANDLF/data/patch_miner/opm/patch_manager.py Outdated Show resolved Hide resolved
GANDLF/data/patch_miner/opm/patch_manager.py Outdated Show resolved Hide resolved
shubhaminnani and others added 8 commits May 19, 2023 15:49
Co-authored-by: Sarthak Pati <sarthak.pati@pennmedicine.upenn.edu>
Co-authored-by: Sarthak Pati <sarthak.pati@pennmedicine.upenn.edu>
Co-authored-by: Sarthak Pati <sarthak.pati@pennmedicine.upenn.edu>
Co-authored-by: Sarthak Pati <sarthak.pati@pennmedicine.upenn.edu>
Co-authored-by: Sarthak Pati <sarthak.pati@pennmedicine.upenn.edu>
Co-authored-by: Sarthak Pati <sarthak.pati@pennmedicine.upenn.edu>
Co-authored-by: Sarthak Pati <sarthak.pati@pennmedicine.upenn.edu>
Co-authored-by: Sarthak Pati <sarthak.pati@pennmedicine.upenn.edu>
@sarthakpati
Copy link
Collaborator

MLCommons CLA bot: Thank you very much for your submission, we really appreciate it. Before we can accept your contribution, we ask that you sign the MLCommons CLA (Apache 2). Please use this [Google form] (https://forms.gle/Ew1KkBVpyeJDuRw67) to initiate authorization. If you are from an MLCommons member organization, we will request that you be added to the CLA. If you are not from a member organization, we will email you a CLA to sign. For any questions, please contact support@mlcommons.org.0 out of 1 committers have signed the MLCommons CLA.❌ @shubhaminnaniYou can retrigger this bot by commenting recheck in this Pull Request

Thanks for the contribution, @shubhaminnani! Let me know once you have signed and submitted the CLA, and I can re-check this.

@sarthakpati sarthakpati changed the title patch level curation of the tile Adding mechanism to curate each extracted patch May 19, 2023
@codecov
Copy link

codecov bot commented May 19, 2023

Codecov Report

Merging #653 (dad52e2) into master (e6cec23) will increase coverage by 0.02%.
The diff coverage is 90.90%.

❗ Current head dad52e2 differs from pull request most recent head 68e00a3. Consider uploading reports for the commit 68e00a3 to get more accurate results

@@            Coverage Diff             @@
##           master     #653      +/-   ##
==========================================
+ Coverage   94.60%   94.62%   +0.02%     
==========================================
  Files         116      116              
  Lines        8038     8056      +18     
==========================================
+ Hits         7604     7623      +19     
+ Misses        434      433       -1     
Flag Coverage Δ
unittests 94.62% <90.90%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
GANDLF/data/patch_miner/opm/patch_manager.py 91.19% <ø> (ø)
GANDLF/data/patch_miner/opm/utils.py 94.53% <88.88%> (-0.62%) ⬇️
GANDLF/cli/patch_extraction.py 95.65% <100.00%> (+6.52%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Collaborator

@sarthakpati sarthakpati left a comment

Choose a reason for hiding this comment

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

This will be a major round of changes that stems from the fact that there is no unit test coverage for this code addition. Going back from there, I am seeing a bunch of places where the code is brittle and can cause breaks/crashes.

GANDLF/data/patch_miner/opm/patch_manager.py Outdated Show resolved Hide resolved
GANDLF/data/patch_miner/opm/patch_manager.py Outdated Show resolved Hide resolved
GANDLF/data/patch_miner/opm/patch_manager.py Outdated Show resolved Hide resolved
GANDLF/data/patch_miner/opm/patch_manager.py Outdated Show resolved Hide resolved
GANDLF/data/patch_miner/opm/patch_manager.py Outdated Show resolved Hide resolved
GANDLF/data/patch_miner/opm/patch_manager.py Outdated Show resolved Hide resolved
sarthakpati and others added 4 commits May 20, 2023 12:32
Co-authored-by: Sarthak Pati <sarthak.pati@pennmedicine.upenn.edu>
Co-authored-by: Sarthak Pati <sarthak.pati@pennmedicine.upenn.edu>
@sarthakpati
Copy link
Collaborator

Hey @shubhaminnani, I have converted this PR to draft until it is ready for review once again (at least once the tests are all passing). All the best! 👍🏽

@shubhaminnani shubhaminnani marked this pull request as ready for review July 5, 2023 18:27
Copy link
Collaborator

@sarthakpati sarthakpati left a comment

Choose a reason for hiding this comment

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

Minor changes requested.

GANDLF/data/patch_miner/opm/utils.py Outdated Show resolved Hide resolved
GANDLF/data/patch_miner/opm/utils.py Outdated Show resolved Hide resolved
shubhaminnani and others added 2 commits July 5, 2023 16:13
Co-authored-by: Sarthak Pati <sarthak.pati@pennmedicine.upenn.edu>
sarthakpati
sarthakpati previously approved these changes Jul 5, 2023
Copy link
Collaborator

@sarthakpati sarthakpati left a comment

Choose a reason for hiding this comment

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

LGTM pending tests.

Copy link
Collaborator

@sarthakpati sarthakpati left a comment

Choose a reason for hiding this comment

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

LGTM

@sarthakpati sarthakpati merged commit 0738c92 into mlcommons:master Jul 6, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Jul 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

code for curation of tiles after patching
3 participants