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

⚡ Improve Engine Performance and Implementation #578

Draft
wants to merge 126 commits into
base: develop
Choose a base branch
from

Conversation

shaneahmed
Copy link
Member

@shaneahmed shaneahmed commented Mar 31, 2023

  • Improve Engines performance and implementation
  • Redesigns PatchPredictor engine using the new EngineABC base class.
  • The WSIs are now processed using the same code as for the processing the patches using WSI based dataloader.
  • The intermediate output is saved as zarr for the WSIs to resolve memory issues.
  • The output of model architectures should now be a dictionary.
  • The output can be specified as AnnotationStore for visualisation using TIAViz.
  • Fix mypy Type Checks for cli/common.py
  • Redesigns PatchPredictor engine using the new EngineABC base class.
  • The WSIs are now processed using the same code as for the processing the patches using WSI based dataloader.
  • The intermediate output is saved as zarr for the WSIs to resolve memory issues.
  • The output of model architectures should now be a dictionary.
  • The output can be specified as AnnotationStore for visualisation using TIAViz.
  • Add PatchPredictor Engine based on EngineABC
  • Add return_probabilities option to Params
  • Removes merge_predictions option in PatchPredictor engine.
  • Defines post_process_cache_mode which allows running the algorithm on WSI
  • Add infer_wsi for WSI inference
  • Removes save_wsi_output as this is not required after post processing.
  • Removes merge_predictions and fixes docstring in EngineABCRunParams
  • compile_model is now moved to EngineABC init
  • Fixes bug with _calculate_scale_factor
  • Fixes a bug in class_dict definition.
  • _get_zarr_array is now a public function get_zarr_array in misc
  • patch_predictions_as_annotations runs the loop on patch_coords instead of class_probs

@shaneahmed shaneahmed self-assigned this Mar 31, 2023
@shaneahmed shaneahmed added the enhancement New feature or request label Mar 31, 2023
@codecov
Copy link

codecov bot commented Mar 31, 2023

Codecov Report

Attention: Patch coverage is 83.45865% with 110 lines in your changes missing coverage. Please review.

Project coverage is 91.53%. Comparing base (8ff4f5e) to head (e93d98a).

Files with missing lines Patch % Lines
tiatoolbox/models/dataset/dataset_abc.py 70.99% 38 Missing ⚠️
tiatoolbox/models/engine/io_config.py 56.16% 32 Missing ⚠️
tiatoolbox/utils/misc.py 57.14% 26 Missing and 1 partial ⚠️
tiatoolbox/models/engine/semantic_segmentor.py 29.41% 12 Missing ⚠️
tiatoolbox/cli/nucleus_instance_segment.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #578      +/-   ##
===========================================
- Coverage    99.90%   91.53%   -8.38%     
===========================================
  Files           69       71       +2     
  Lines         8715     8929     +214     
  Branches      1149     1184      +35     
===========================================
- Hits          8707     8173     -534     
- Misses           3      742     +739     
- Partials         5       14       +9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

- Refactor engines_abc.py
@shaneahmed shaneahmed changed the title ⚡ Improve Engines Performance and Implementation ⚡ Improve Engine Performance and Implementation Apr 28, 2023
shaneahmed and others added 5 commits November 15, 2024 18:04
-  Add `PatchPredictor` Engine based on `EngineABC`
- Add `return_probabilities` option to Params
- Removes `merge_predictions` option in `PatchPredictor` engine.
- Defines `post_process_cache_mode` which allows running the algorithm on `WSI`
- Add `infer_wsi` for WSI inference
- Removes `save_wsi_output` as this is not required after post processing.
- Removes `merge_predictions` and fixes docstring in EngineABCRunParams
- `compile_model` is now moved to EngineABC init
- Fixes bug with `_calculate_scale_factor`
- Fixes a bug in `class_dict` definition.
- `_get_zarr_array` is now a public function `get_zarr_array` in `misc`
- `patch_predictions_as_annotations` runs the loop on `patch_coords` instead of `class_probs`

---------

Co-authored-by: Mark Eastwood <[email protected]>
Co-authored-by: Mostafa Jahanifar <[email protected]>
Co-authored-by: Adam Shephard <[email protected]>
Co-authored-by: Jiaqi-Lv <[email protected]>
shaneahmed added a commit that referenced this pull request Nov 21, 2024
- Merge changes from #578 which will improve the performance of main branch.
- It will also help simplify #578
shaneahmed added a commit that referenced this pull request Nov 21, 2024
- Add changes from New engine design #578. This will not only simplify the PR but also keep the main repo up to date.
- Refactor `model_to` to `model_abc`
- Instead of `on_gpu` use `device` as an input in line with `PyTorch`.
- `infer_batch` uses `device` as an input instead of `on_gpu`
mbasheer04 added a commit to mbasheer04/tiatoolbox that referenced this pull request Jan 24, 2025
commit d315a5f415542890f351ddb07c771d62c278c2f2
Merge: dab2693 1a27178
Author: Musraf Basheer <[email protected]>
Date:   Fri Jan 24 15:07:28 2025 +0000

    Merge branch 'sam-architecture' into develop

commit dab2693
Merge: ca37400 8ff4f5e
Author: mbasheer04 <[email protected]>
Date:   Fri Jan 24 15:04:28 2025 +0000

    Merge branch 'TissueImageAnalytics:develop' into develop

commit 1a27178
Merge: 3b51acb 8ff4f5e
Author: mbasheer04 <[email protected]>
Date:   Fri Jan 24 14:57:22 2025 +0000

    Merge branch 'TissueImageAnalytics:develop' into sam-architecture

commit 3b51acb
Author: Musraf Basheer <[email protected]>
Date:   Fri Jan 24 14:53:06 2025 +0000

    Improved Engine

commit 8ff4f5e
Author: Mark Eastwood <[email protected]>
Date:   Thu Jan 23 10:03:43 2025 +0000

    📌 Pin `zarr<3.0.0` (TissueImageAnalytics#905)

    - `zarr>3.0.0` is not compatible yet

commit 46d30f2
Merge: 1e136c5 ca37400
Author: Musraf Basheer <[email protected]>
Date:   Fri Jan 17 14:22:18 2025 +0000

    Merge branch 'develop' into sam-architecture

commit ca37400
Merge: 32cae0b f385ced
Author: mbasheer04 <[email protected]>
Date:   Fri Jan 17 14:18:47 2025 +0000

    Merge pull request #1 from TissueImageAnalytics/develop

    Merge develop

commit 1e136c5
Merge: 47a2190 bb9399d
Author: Musraf Basheer <[email protected]>
Date:   Fri Jan 17 14:12:52 2025 +0000

    Merge branch 'sam-architecture' of https://github.com/mbasheer04/tiatoolbox into sam-architecture

commit 47a2190
Author: Musraf Basheer <[email protected]>
Date:   Fri Jan 17 14:08:11 2025 +0000

    Improved engine & added unit tests

commit f385ced
Author: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Date:   Fri Jan 10 12:04:52 2025 +0000

    :technologist: pre-commit autoupdate (TissueImageAnalytics#902)

    * 🧑‍💻 pre-commit autoupdate

    updates:
    - [github.com/executablebooks/mdformat: 0.7.19 → 0.7.21](hukkin/mdformat@0.7.19...0.7.21)
    - [github.com/astral-sh/ruff-pre-commit: v0.8.2 → v0.8.6](astral-sh/ruff-pre-commit@v0.8.2...v0.8.6)

    * [pre-commit.ci] auto fixes from pre-commit.com hooks

    for more information, see https://pre-commit.ci

    ---------

    Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

commit 0ddd2b1
Author: Shan E Ahmed Raza <[email protected]>
Date:   Thu Dec 12 14:45:39 2024 +0000

    🔖 Release 1.6.0 (TissueImageAnalytics#898)

    ## TIAToolbox v1.6.0 (2024-12-12)

    ### Major Updates and Feature Improvements

    - **Foundation Models Support via `timm` API** (TissueImageAnalytics#856, contributed by @GeorgeBatch)
      - Introduced `TimmBackbone` for running additional PyTorch Image Models.
      - Tested models include `UNI`, `Prov-GigaPath`, and `H-optimus-0`.
      - Added an example notebook demonstrating feature extraction with foundation models.
      - `timm` added as a dependency.
    - **Performance Enhancements with `torch.compile`** (TissueImageAnalytics#716)
      - Improved performance on newer GPUs using `torch.compile`.
    - **Multichannel Input Support in `WSIReader`** (TissueImageAnalytics#742)
    - **AnnotationStore Filtering for Patch Extraction** (TissueImageAnalytics#822)
    - **Python 3.12 Support**
    - **Deprecation of Python 3.8 Support**
    - **CLI Response Time Improvements** (TissueImageAnalytics#795)

    ### API Changes

    - **Device Specification Update** (TissueImageAnalytics#882)
      - Replaced `has_gpu` with `device` for specifying GPU or CPU usage, aligning with PyTorch's `Model.to()` functionality.
    - **Windows Compatibility Enhancement** (TissueImageAnalytics#769)
      - Replaced `POWER` with explicit multiplication.

    ### Bug Fixes and Other Changes

    - **TIFFWSIReader Bound Reading Adjustment** (TissueImageAnalytics#777)
      - Fixed `read_bound` to use adjusted bounds.
      - Reduced code complexity in `WSIReader` (TissueImageAnalytics#814).
    - **Annotation Rendering Fixes** (TissueImageAnalytics#813)
      - Corrected rendering of annotations with holes.
    - **Non-Tiled TIFF Support in `WSIReader`** (TissueImageAnalytics#807, contributed by @GeorgeBatch)
    - **HoVer-Net Documentation Update** (TissueImageAnalytics#751)
      - Corrected class output information.
    - **Citation File Fix for `cffconvert`** (TissueImageAnalytics#869, contributed by @Alon-Alexander)
    - **Bokeh Compatibility Updates**
      - Updated `bokeh_app` for compatibility with `bokeh>=3.5.0`.
      - Switched from `size` to `radius` for `bokeh>3.4.0` compatibility (TissueImageAnalytics#796).
    - **JSON Extraction Fixes** (TissueImageAnalytics#772)
      - Restructured SQL expression construction for JSON properties with dots in keys.
    - **VahadaneExtractor Warning** (TissueImageAnalytics#871)
      - Added warning due to changes in `scikit-learn>0.23.0` dictionary learning (TissueImageAnalytics#382).
    - **PatchExtractor Error Message Refinement** (TissueImageAnalytics#883)
    - **Immutable Output Fix in `WSIReader`** (TissueImageAnalytics#850)

    ### Development-Related Changes

    - **Mypy Checks Added**
      - Applied to `utils`, `tools`, `data`, `annotation`, and `cli/common`.
    - **ReadTheDocs PDF Build Deprecation**
    - **Formatter Update**
      - Replaced `black` with `ruff-format`.
    - **Dependency Removal**
      - Removed `jinja2`.
    - **Test Environment Update**
      - Updated to `Ubuntu 24.04`.
    - **Conda Environment Workflow Update**
      - Implemented `micromamba` setup.
    - **Codecov Reporting Fix** (TissueImageAnalytics#811)
      **Full Changelog:** TissueImageAnalytics/tiatoolbox@v1.5.1...v1.6.0

commit a38d95f
Author: Shan E Ahmed Raza <[email protected]>
Date:   Thu Dec 5 17:14:20 2024 +0000

    [skip ci] 🐛 Fix `benchmarks/annotation_store.ipynb` (TissueImageAnalytics#894)

    - Fix `benchmarks/annotation_store.ipynb` errors

commit 6b214fe
Author: adamshephard <[email protected]>
Date:   Tue Dec 3 14:09:33 2024 +0000

    📝 Add Example Notebook for Foundation Models (TissueImageAnalytics#887)

    - Add Example Notebook to explain how to use Foundation Models from `timm` module in TIAToolbox.

    ---------

    Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
    Co-authored-by: Shan E Ahmed Raza <[email protected]>

commit 442bd3f
Author: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Date:   Tue Dec 3 09:40:40 2024 +0000

    :technologist: pre-commit autoupdate (TissueImageAnalytics#891)

    <!--pre-commit.ci start-->
    updates:
    - [github.com/executablebooks/mdformat: 0.7.18 → 0.7.19](hukkin/mdformat@0.7.18...0.7.19)

    ---------

    Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
    Co-authored-by: Shan E Ahmed Raza <[email protected]>

commit 741463c
Author: Shan E Ahmed Raza <[email protected]>
Date:   Mon Dec 2 22:45:05 2024 +0000

    :pushpin: Update `ruff` Version to `v0.8.1` (TissueImageAnalytics#890)

    - Update `ruff` Version to `v0.8.1`

    ---------

    Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

commit 9a62c10
Author: Shan E Ahmed Raza <[email protected]>
Date:   Mon Dec 2 17:11:39 2024 +0000

    [skip ci] :memo: Update Jupyter Notebooks for Release `v1.6.0` (TissueImageAnalytics#885)

    - Update Jupyter Notebooks for the New Release
    - Fix issues with API changes e.g., device instead of ON_GPU flag.

commit 4a1940d
Author: Shan E Ahmed Raza <[email protected]>
Date:   Fri Nov 29 16:13:37 2024 +0000

    :technologist: `torch.compile` is not compatible with Windows. (TissueImageAnalytics#888)

    - `torch.compile` is not currently compatible with Windows. See pytorch/pytorch#122094

commit 5f1cecb
Author: Jiaqi-Lv <[email protected]>
Date:   Fri Nov 29 14:37:39 2024 +0000

    🧑‍💻 Refine `PatchExtractor` Error Message (TissueImageAnalytics#883)

    - Fix Misleading error message TissueImageAnalytics#881

commit 5beb119
Author: Musraf Basheer <[email protected]>
Date:   Tue Nov 26 18:47:14 2024 +0000

    Initialised SAM Engine

commit bb9399d
Author: mbasheer04 <[email protected]>
Date:   Fri Nov 22 19:26:45 2024 +0000

    Created using Colab

commit ca13e7f
Author: Shan E Ahmed Raza <[email protected]>
Date:   Thu Nov 21 22:28:44 2024 +0000

    ♻️ Update Changes from New Engine Design (TissueImageAnalytics#882)

    - Add changes from New engine design TissueImageAnalytics#578. This will not only simplify the PR but also keep the main repo up to date.
    - Refactor `model_to` to `model_abc`
    - Instead of `on_gpu` use `device` as an input in line with `PyTorch`.
    - `infer_batch` uses `device` as an input instead of `on_gpu`

commit 91ca877
Merge: 1e27436 32cae0b
Author: mbasheer04 <[email protected]>
Date:   Thu Nov 21 21:03:41 2024 +0000

    Merge branch 'develop' of https://github.com/TissueImageAnalytics/tiatoolbox into sam-architecture

commit 1e27436
Author: mbasheer04 <[email protected]>
Date:   Thu Nov 21 21:00:34 2024 +0000

    Implementing architecture for full-image segmentation

commit 32cae0b
Author: Abdol <[email protected]>
Date:   Fri Nov 15 20:29:34 2024 +0000

    ⚡️Add `torch.compile` Functionality (TissueImageAnalytics#716)

    - Integrates PyTorch 2.0's [torch.compile](https://pytorch.org/docs/stable/generated/torch.compile.html) functionality to demonstrate performance improvements in torch code. This PR focuses on adding `torch.compile` to `PatchPredictor`.

    **Notes:**
    - According to the [documentation](https://pytorch.org/tutorials/intermediate/torch_compile_tutorial.html), noticeable performance can be achieved when using modern NVIDIA GPUs (H100, A100, or V100)

    **TODO:**
    - [x] Resolve compilation errors related to using `torch.compile` in running models
    - [x] Initial config
    - [x] Add to patch predictor
    - [x] Add to registration
    - [x] Add to segmentation
    - [x] Test on custom models
    - [x] Test on `torch.compile` compatible GPUs

    ---------

    Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
    Co-authored-by: Shan E Ahmed Raza <[email protected]>
    Co-authored-by: Jiaqi-Lv <[email protected]>

commit e1a57c6
Author: mbasheer04 <[email protected]>
Date:   Fri Nov 15 17:29:44 2024 +0000

    Created Jupyter Notebook for SAM architecture

commit 9113996
Author: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Date:   Fri Nov 15 17:18:16 2024 +0000

    :technologist: pre-commit autoupdate (TissueImageAnalytics#880)

    * 🧑‍💻 pre-commit autoupdate

    updates:
    - [github.com/astral-sh/ruff-pre-commit: v0.7.0 → v0.7.2](astral-sh/ruff-pre-commit@v0.7.0...v0.7.2)

    * 📌 Update `ruff` version.

    ---------

    Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
    Co-authored-by: Shan E Ahmed Raza <[email protected]>

commit c980eec
Author: George Batchkala <[email protected]>
Date:   Fri Nov 15 16:31:42 2024 +0000

    🆕 Integrate Foundation Models Available VIA `timm`: `UNI`, `Prov-GigaPath`, `H-optimus-0` (TissueImageAnalytics#856)

    - Integrates pre-trained foundation models from other labs into tiatoolbox.models.architecture.vanilla.py.

    Currently, the `_get_architecture()` function allows the use of models from `torchvision.models`.

    A new function `_get_timm_architecture()` has been incorporated to support foundation models which are available from `timm` with weights on HuggingFace Hub.

    - All the models from `timm` that used require users to sign the licence agreement with the authors.
    - The users can add new models by modifying `_get_timm_architecture()`

    ---------

    Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
    Co-authored-by: Shan E Ahmed Raza <[email protected]>

commit 7fb9926
Author: Mostafa Jahanifar <[email protected]>
Date:   Fri Nov 8 17:25:02 2024 +0000

    🐛 Add Warning for `VahadaneExtractor` Algorithm Instability (TissueImageAnalytics#871)

    - Adds a warning to the `VahadaneExtractor` to inform users about the algorithm's instability due to changes in the dictionary learning algorithm in `scikit-learn versions > 0.23.0 (see issue TissueImageAnalytics#382)`.
    - The docstrings are updated accordingly to reflect this warning.
    - No other functionality is altered.

    ---------

    Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

commit f4f2fa6
Author: mbasheer04 <[email protected]>
Date:   Sun Nov 3 23:41:50 2024 +0000

    Initial setup of SAM architecture

commit 12d435e
Author: Shan E Ahmed Raza <[email protected]>
Date:   Thu Oct 24 16:40:56 2024 +0100

    :pushpin: Pin `numpy<2.0.0` (TissueImageAnalytics#876)

    - `numpy>=2.0` breaks the tests.

commit 76f02f8
Author: Mark Eastwood <[email protected]>
Date:   Fri Oct 18 14:53:49 2024 +0100

    🆕 Use Annotations as a Filter for Patch Extraction (TissueImageAnalytics#822)

    - Adds the ability to get patches filtered by class or label using AnnotationStore.
    - Allows the user to provide a path to an annotation store as the mask.
      - By default the mask will then be built out of all the annotations, but the user can also provide any filter that works in the annotation store, and have only those annotations make the mask.

    For example, if the user have some pathologist annotations with some regions, and they would only want to extract patches which have some overlap with regions labelled 'tumor' or any other class, they would just need to provide the appropriate string ("props['label'] == 'tumor'" for example).

    ---------

    Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

commit 0b857c7
Author: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Date:   Fri Oct 18 12:00:04 2024 +0100

    :technologist: `pre-commit` autoupdate (TissueImageAnalytics#868)

    * [pre-commit.ci] pre-commit autoupdate

    updates:
    - [github.com/pre-commit/pre-commit-hooks: v4.6.0 → v5.0.0](pre-commit/pre-commit-hooks@v4.6.0...v5.0.0)
    - [github.com/astral-sh/ruff-pre-commit: v0.6.5 → v0.6.9](astral-sh/ruff-pre-commit@v0.6.5...v0.6.9)

    * 🐛 Fix pre-commit yaml

    * 🧑‍💻 Change to monthly schedule.

    * [pre-commit.ci] pre-commit autoupdate

    updates:
    - [github.com/executablebooks/mdformat: 0.7.17 → 0.7.18](hukkin/mdformat@0.7.17...0.7.18)
    - [github.com/pre-commit/pre-commit-hooks: v4.6.0 → v5.0.0](pre-commit/pre-commit-hooks@v4.6.0...v5.0.0)
    - [github.com/astral-sh/ruff-pre-commit: v0.6.5 → v0.6.9](astral-sh/ruff-pre-commit@v0.6.5...v0.6.9)

    * 🔀 Merge online and local copies

    * 📌 Pin dependencies

    * 📌 Pin `ruff` version

    ---------

    Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
    Co-authored-by: Shan E Ahmed Raza <[email protected]>

commit 52792bc
Author: Alon Alexander <[email protected]>
Date:   Wed Oct 2 12:51:52 2024 +0300

    🐛 Fix `CITATION.cff` to Make it Compatible with `cffconvert`

    - To cite this repository using `cffconvert`, the conversion fails as the citation file was invalid.
    - This PR fixes the citation file to a valid ORCID.

commit 3714ef7
Author: Shan E Ahmed Raza <[email protected]>
Date:   Wed Oct 2 10:22:26 2024 +0100

    :pushpin: Pin `bokeh` version to less than 3.6.0 (TissueImageAnalytics#870)

    - Pin `bokeh` version to less than `3.6.0`
    - `bokeh` 3.6.0 has breaking changes
* 🐛 Fix PLC0206
- Fix PLC0206 Extracting value from dictionary without calling `.items()`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment