Implement Preparation progress dashboard utility script #579
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
aristizabal95
added
type: enhancement
aristizabal95
had a problem deploying
to
testing-external-code
GitHub Actions
Failure
|
MLCommons CLA bot All contributors have signed the MLCommons CLA ✍️ ✅ |
aristizabal95
had a problem deploying
to
testing-external-code
GitHub Actions
Failure
aristizabal95
had a problem deploying
to
testing-external-code
GitHub Actions
Failure
aristizabal95
had a problem deploying
to
testing-external-code
GitHub Actions
Failure
aristizabal95
had a problem deploying
to
testing-external-code
GitHub Actions
Failure
aristizabal95
had a problem deploying
to
testing-external-code
GitHub Actions
Failure
aristizabal95
had a problem deploying
to
testing-external-code
GitHub Actions
Failure
…l95/medperf-2 into preparation-dashboard
aristizabal95
had a problem deploying
to
testing-external-code
GitHub Actions
Failure
aristizabal95
had a problem deploying
to
testing-external-code
GitHub Actions
Failure
aristizabal95
had a problem deploying
to
testing-external-code
GitHub Actions
Failure
hasan7n
reviewed
May 3, 2024
hasan7n
reviewed
May 3, 2024
Comment on lines
+18
to
+20
| for dset in dsets: | ||
| user_id = dset["owner"] | ||
| dset["user"] = config.comms.get_user(user_id) |
There was a problem hiding this comment.
how many API calls are we expecting here? maybe we can have an endpoint that returns all of this
There was a problem hiding this comment.
I expect here a large number of unnecessary API calls (this includes multiple datasets for the same user, so we are talking more than 30 maybe?)
let's have an issue to replace this code later. in the FL PR we will have an endpoint that lists experiment datasets and their owners information so we can reuse the logic defined there to make this better.
hasan7n
reviewed
May 3, 2024
Comment on lines
14
to
37
| -5,Manual Review Required,"Baseline tumor segmentations have been generated. Manual corrections are required to proceed. Here are the following recommended methods. | ||
|
|
||
| ### Local Manual Corrections | ||
| If you're doing manual corrections on this machine, and you're running the manual Data Preparation pipeline locally, you may find buttons at the bottom of this page to aid you in the process of manual correction. These buttons automatically start the review process with ITK-Snap using the generated tumor segmentation mask. Please ensure ITK-Snap has been installed with client-line tools to use this method. You may edit the file as many times as needed. Once you're done with this subject, press the ""Mark as finalized"" button to let the pipeline know the subject is done. | ||
|
|
||
| In some cases, local manual correction is not possible, or the default baseline segmentation is not sufficiently good to work with. In those cases, it is recommended to do manual correction through the **Packaging method**. | ||
|
|
||
| ### Packaging Method | ||
| If using the monitoring tool, you may package cases for review by pressing the 'Package cases for review' button under **SUMMARY**. This will create a tarball file on the working directory (displayed on the monitoring tool header), containing all the cases for review. You may untar the segmentation with the following command | ||
|
|
||
| ``` tar -xzf review_cases.tar.gz ``` | ||
|
|
||
| If working remotely, move the tarball file to your local machine. Untar the tarball file, select the best baseline segmentation and make any necessary corrections. Once you're done, move the finalized file to the `finalized` folder, retaining the original name. | ||
|
|
||
| If using the monitoring tool, create a new tarball with the previously untarred files, as well as finalized cases. You can do so with the following command | ||
|
|
||
| ``` tar -czf reviewed_cases.tar.gz -C review_cases . ``` | ||
|
|
||
| Place this new tarball file on the directory the monitoring tool is running (displayed on the monitoring tool header). The tool will automatically detect the finalized cases and place them in the expected locations. | ||
|
|
||
| ### Brain Mask Correction | ||
| There might be situations where the brain mask is not correct. If working locally you may use the `Review Brain Mask` button to automatically make brain mask corrections with ITK-Snap. If instead you're using the previously menitoned tarball, you may find the brain mask segmentation as `brainMask_fused.nii.gz`. Please make the necessary corrections under that file, overwriting the contents. Once the new tarball is created and placed within the directory the monitoring tool is running, it will be automatically identified and placed in the correct location. | ||
| **NOTE** Modifying the brain mask invalidates the tumor segmentations, and re-runs the tumor segmentation procedure using this new mask. | ||
| ",,mediumseagreen |
There was a problem hiding this comment.
is the text here intentional?
There was a problem hiding this comment.
It seems to me this is just a multiline str value of comment field for the -5 stage. At least csv is parsed successfully
There was a problem hiding this comment.
And I don't see any usage of this comment field in the dashboard. I believe it is here just because stages.csv file was taken as it is used in monitor (?). Anyway, it does not break anything
|
@aristizabal95 I left few comments. |
aristizabal95
had a problem deploying
to
testing-external-code
GitHub Actions
Failure
aristizabal95
had a problem deploying
to
testing-external-code
GitHub Actions
Failure
aristizabal95
had a problem deploying
to
testing-external-code
GitHub Actions
Failure
aristizabal95
had a problem deploying
to
testing-external-code
GitHub Actions
Failure
VukW
reviewed
May 9, 2024
VukW
reviewed
May 9, 2024
VukW
reviewed
May 9, 2024
VukW
previously approved these changes
May 9, 2024
Co-authored-by: Viacheslav Kukushkin <[email protected]>
aristizabal95
had a problem deploying
to
testing-external-code
GitHub Actions
Failure
…l95/medperf-2 into preparation-dashboard
aristizabal95
had a problem deploying
to
testing-external-code
GitHub Actions
Failure
aristizabal95
had a problem deploying
to
testing-external-code
GitHub Actions
Failure
aristizabal95
had a problem deploying
to
testing-external-code
GitHub Actions
Failure
VukW
approved these changes
May 10, 2024
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR includes the necessary changes for running a preparation dashboard for their mlcube/experiment. Although at the moment is somewhat targeted to RANO, small tweaks may be done to make it generalizable.
This PR includes two things: