-
Notifications
You must be signed in to change notification settings - Fork 79
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 MLCube wrapper for metrics API #681
Conversation
MLCommons CLA bot All contributors have signed the MLCommons CLA ✍️ ✅ |
Codecov Report
@@ Coverage Diff @@
## master #681 +/- ##
=======================================
Coverage 94.59% 94.60%
=======================================
Files 116 116
Lines 8017 8038 +21
=======================================
+ Hits 7584 7604 +20
- Misses 433 434 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1 file with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
@AlexanderGetka-cbica @sarthakpati Please start reviewing. I am left with adding a unittest and modifying documentation if necessary. Also a question: do you know if Dev-COntainer CI check fails randomly? I think so. |
Is it possible to combine
I have re-initiated the test. Haven't seen that happening before, TBH. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor changes, but the major one if gandlf_deployMetrics
and gandlf_deploy
can be combined.
I agree. I split them because I though it would be confusing for the user to see many optional args depending on the mlcube type they want to deploy. But on a second thought, I guess it should be fine with some good documentation |
Co-authored-by: Sarthak Pati <sarthak.pati@pennmedicine.upenn.edu>
Co-authored-by: Sarthak Pati <sarthak.pati@pennmedicine.upenn.edu>
@sarthakpati @AlexanderGetka-cbica I think this PR is done from my side. I also tested a generated metrics MLCube with MedPerf and its compatibility with a model created from GaNDLF. All is working. I see two tests failing: any idea? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor changes, should be good to go once we figure out what's going on with devcontainers/features#598
#683 should take care of this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Description: TBD
Proposed Changes
cli/deploy.py
to reuse code for deploying GaNDLF as a metrics calculator MLCubegandlf_deployMetrics
command. This command will accept a custom entrypoint script that do some preprocessing of inputs before calling GaNDLF's generate metrics command. One usecase (for MedPerf) is if input predictions and labels are expected to come from different MLCube mount points, the MLCube author can usegandlf_deployMetrics
and pass to it a custom script that first combines predictions and labels to form a single data input csv file and then call thegandlf_generateMetrics
command. Example scripts are provided inmlcube/metrics_mlcube
directory.Checklist
CONTRIBUTING
guide.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].