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

Add check for Phase correlation transform #645

Merged
merged 6 commits into from
Jul 20, 2023
Merged

Conversation

ruqayya
Copy link
Contributor

@ruqayya ruqayya commented Jul 19, 2023

Likewise other intermediate transform, Phase cross-correlation transform should only be applied if it improves the DICE overlap.

@ruqayya ruqayya requested a review from shaneahmed July 19, 2023 16:27
@shaneahmed shaneahmed added this to the Release v1.5.0 milestone Jul 19, 2023
Copy link
Member

@shaneahmed shaneahmed left a comment

Choose a reason for hiding this comment

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

I think as there is an if statement you may need to add a test.
You can create a new function that would help to add a quick test.

@ruqayya
Copy link
Contributor Author

ruqayya commented Jul 20, 2023

I think as there is an if statement you may need to add a test. You can create a new function that would help to add a quick test.

'If statement' is already covered by the existing test functions such as test_register_output_with_initializer. The phase cross correlation transform for the images that we are using for testing results in low dice value as compared to previous dice value and hence "if statement" is executed on calling register function. Do we still need to have a separate function?

@shaneahmed
Copy link
Member

I think as there is an if statement you may need to add a test. You can create a new function that would help to add a quick test.

'If statement' is already covered by the existing test functions such as test_register_output_with_initializer. The phase cross correlation transform for the images that we are using for testing results in low dice value as compared to previous dice value and hence "if statement" is executed on calling register function. Do we still need to have a separate function?

no if it is already covered then it is fine. As it is a new change, I thought it might be required. Once tests are passed we will know if it is covered.

@codecov
Copy link

codecov bot commented Jul 20, 2023

Codecov Report

Merging #645 (6aef433) into develop (3614f61) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##           develop     #645    +/-   ##
=========================================
  Coverage    99.78%   99.78%            
=========================================
  Files           65       65            
  Lines         7075     7079     +4     
  Branches      1397     1160   -237     
=========================================
+ Hits          7060     7064     +4     
  Misses           7        7            
  Partials         8        8            
Impacted Files Coverage Δ
tiatoolbox/tools/registration/wsi_registration.py 100.00% <100.00%> (ø)

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

Copy link
Member

@shaneahmed shaneahmed left a comment

Choose a reason for hiding this comment

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

Thanks @ruqayya

@shaneahmed shaneahmed merged commit 7c12443 into develop Jul 20, 2023
@shaneahmed shaneahmed deleted the bug-fix-registration branch July 20, 2023 09:51
@shaneahmed shaneahmed mentioned this pull request Dec 15, 2023
shaneahmed added a commit that referenced this pull request Dec 15, 2023
## 1.5.0 (2023-12-15)

### Major Updates and Feature Improvements

- Adds the bokeh visualization tool. #684
  - The tool allows a user to launch a server on their machine to visualise whole slide images, overlay the results of deep learning algorithms or to select a patch from whole slide image and run TIAToolbox deep learning engines.
  - This tool powers the TIA demos server. For details please see https://tiademos.dcs.warwick.ac.uk/.
- Extends Annotation to Support Init from WKB #639
- Adds `IOConfig` for NuClick in `pretrained_model.yaml` #709
- Adds functions to save the TIAToolbox Engine outputs to Zarr and AnnotationStore files. #724
- Adds Support for QuPath Annotation Imports #721

### Changes to API

- Adds `model.to(device)` and `model.load_model_from_file()` functionality to make it compatible with PyTorch API. #733
- Replaces `pretrained` with `weights` to make the engines compatible with the new PyTorch API. #621
- Adds support for high-level imports for various utility functions and classes such as `WSIReader`, `PatchPredictor` and `imread` #606, #607,
- Adds `tiatoolbox.typing` for type hints. #619
- Fixes incorrect file size saved by `save_tiles`, issue with certain WSIs raised by @TomastpPereira
- TissueMasker transform now returns mask instead of a list. #748
  - Fixes #732

### Bug Fixes and Other Changes

- Fixes `pixman` incompability error on Colab #601
- Removes `shapely.speedups`. The module no longer has any affect in Shapely >=2.0. #622
- Fixes errors in the slidegraph example notebook #608
- Fixes bugs in WSI Registration #645, #670, #693
- Fixes the situation where PatchExtractor.get_coords() can return patch coords which lie fully outside the bounds of a slide. #712
  - Fixes #710
- Fixes #738 raised by @xiachenrui

### Development related changes

- Replaces `flake8` and `isort` with `ruff` #625, #666
- Adds `mypy` checks to `root` and `utils` package. This will be rolled out in phases to other modules. #723
- Adds a module to detect file types using magic number/signatures #616
- Uses `poetry` for version updates instead of `bump2version`. #638
- Removes `setup.cfg` and uses `pyproject.toml` for project configurations.
- Reduces runtime for some unit tests e.g., #627, #630, #631, #629
- Reuses models and datasets in tests on GitHub actions by utilising cache #641, #644
- Set up parallel tests locally #671

**Full Changelog:** v1.4.0...v1.5.0

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: mostafajahanifar <74412979+mostafajahanifar@users.noreply.github.com>
Co-authored-by: John Pocock <John-P@users.noreply.github.com>
Co-authored-by: DavidBAEpstein <David.Epstein@warwick.ac.uk>
Co-authored-by: David Epstein <22086916+DavidBAEpstein@users.noreply.github.com>
Co-authored-by: Ruqayya Awan <18444369+ruqayya@users.noreply.github.com>
Co-authored-by: Mark Eastwood <20169086+measty@users.noreply.github.com>
Co-authored-by: adamshephard <39619155+adamshephard@users.noreply.github.com>
Co-authored-by: adamshephard <adam.shephard@warwick.ac.uk>
Co-authored-by: Abdol <a@fkrtech.com>
Co-authored-by: Jiaqi-Lv <60471431+Jiaqi-Lv@users.noreply.github.com>
Co-authored-by: Abishek <abishekraj6797@gmail.com>
Co-authored-by: Dmitrii Blaginin <blaginin@mbp.lan>
@shaneahmed shaneahmed mentioned this pull request Dec 15, 2023
shaneahmed added a commit that referenced this pull request Dec 15, 2023
## 1.5.0 (2023-12-15)

### Major Updates and Feature Improvements

- Adds the bokeh visualization tool. #684
  - The tool allows a user to launch a server on their machine to visualise whole slide images, overlay the results of deep learning algorithms or to select a patch from whole slide image and run TIAToolbox deep learning engines.
  - This tool powers the TIA demos server. For details please see https://tiademos.dcs.warwick.ac.uk/.
- Extends Annotation to Support Init from WKB #639
- Adds `IOConfig` for NuClick in `pretrained_model.yaml` #709
- Adds functions to save the TIAToolbox Engine outputs to Zarr and AnnotationStore files. #724
- Adds Support for QuPath Annotation Imports #721

### Changes to API

- Adds `model.to(device)` and `model.load_model_from_file()` functionality to make it compatible with PyTorch API. #733
- Replaces `pretrained` with `weights` to make the engines compatible with the new PyTorch API. #621
- Adds support for high-level imports for various utility functions and classes such as `WSIReader`, `PatchPredictor` and `imread` #606, #607,
- Adds `tiatoolbox.typing` for type hints. #619
- Fixes incorrect file size saved by `save_tiles`, issue with certain WSIs raised by @TomastpPereira
- TissueMasker transform now returns mask instead of a list. #748
  - Fixes #732

### Bug Fixes and Other Changes

- Fixes `pixman` incompability error on Colab #601
- Removes `shapely.speedups`. The module no longer has any affect in Shapely >=2.0. #622
- Fixes errors in the slidegraph example notebook #608
- Fixes bugs in WSI Registration #645, #670, #693
- Fixes the situation where PatchExtractor.get_coords() can return patch coords which lie fully outside the bounds of a slide. #712
  - Fixes #710
- Fixes #738 raised by @xiachenrui

### Development related changes

- Replaces `flake8` and `isort` with `ruff` #625, #666
- Adds `mypy` checks to `root` and `utils` package. This will be rolled out in phases to other modules. #723
- Adds a module to detect file types using magic number/signatures #616
- Uses `poetry` for version updates instead of `bump2version`. #638
- Removes `setup.cfg` and uses `pyproject.toml` for project configurations.
- Reduces runtime for some unit tests e.g., #627, #630, #631, #629
- Reuses models and datasets in tests on GitHub actions by utilising cache #641, #644
- Set up parallel tests locally #671

**Full Changelog:** v1.4.0...v1.5.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants