✨ NEW: Add Nucleus Detection Engine #538
Conversation
shaneahmed
commented
Feb 16, 2023
shaneahmed
commented
- Add Nucleus Detection Engine
- Fix documentation in patch_predictor.py
- Add Nucleus Detection Engine - Fix documentation in patch_predictor.py
- Improve structure of init
Codecov Report
@@ Coverage Diff @@
## develop #538 +/- ##
===========================================
- Coverage 99.77% 99.63% -0.15%
===========================================
Files 64 63 -1
Lines 6817 6642 -175
Branches 1101 1078 -23
===========================================
- Hits 6802 6618 -184
- Misses 7 15 +8
- Partials 8 9 +1
... and 24 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
- Add documentation for mapde and sccnn - Add a test for nucleus detection - Fix input in patch predictor test
- Fix mpp for cell detection
…-engine # Conflicts: # docs/pretrained.rst # tiatoolbox/data/pretrained_model.yaml
- Fix bug with loading NucleusDetector
- Fix bug with loading sccnn
- Fix test bug in test_patch_predictor.py
- Add test for NucleusDetector
- Set GPU Signed-off-by: Shan E Ahmed Raza <[email protected]>
- Add test to improve coverage
There was a problem hiding this comment.
Thanks, Shan for doing this important PR.
I made some comments. As we mentioned before, I believe this engine requires some special consideration in the design, especially for post-processing. However, if we want to have it in its current shape for now, it needs some changes as well as some more rigorous testing. For example, how does the algorithm work on a slightly large WSI on a mediocre system (16-32GB RAM)?
Also, we need to consider the detection class and probability in the outputs as well because there might be some models that predict these parameters too.
| ) | ||
|
|
||
|
|
||
| class IONucleusDetectorConfig(IOSegmentorConfig): |
There was a problem hiding this comment.
In the current shape, defining this new class seems unnecessary because there is no change to IOSegmentorConfig. I see no reason why we could not use the main class.
There was a problem hiding this comment.
Agree I can remove this for now.
| # Coordinates in output resolution for the current canvas. | ||
| cum_canvas = np.expand_dims(cum_canvas, axis=0) | ||
| coordinates_canvas = pd.DataFrame( | ||
| self.model.postproc_func(cum_canvas), columns=["x", "y"] |
There was a problem hiding this comment.
Now that engine is written like this, the post_proc function should be working with all detection models. There are two major things to consider:
1- this part should be compatible with all the detection models
2- this part should also support prediction class and prediction probability of detections (if available)
Therefore, it seems that we either need to update the postproc function of the detection networks to return the results in a predefined structure ([x, y, cls, prob]) and then convert the results properly here.
There was a problem hiding this comment.
Also, I wonder if the post-processing works with large WSIs? as it seems that results are saved in high resolution and then the whole canvas is passed to the post-processing.
- Add example models
# Conflicts: # tiatoolbox/models/engine/patch_predictor.py # tiatoolbox/models/engine/semantic_segmentor.py
- Pin `torch` version
- Pin `torch` version
- Try cuda 11.8 to run the tests as it passes locally. Signed-off-by: Shan E Ahmed Raza <[email protected]>
