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

Fix overlapping in patches aggregator #832

Merged
merged 1 commit into from
Feb 26, 2022
Merged

Conversation

fepegar
Copy link
Owner

@fepegar fepegar commented Feb 26, 2022

Fixes #813.

Description
This PR fixes a bug in the aggregator. When certain conditions were met, overlapping patches were over-cropped, causing spatial shifting between the input and output volumes during inference with a grid sampler and an aggregator.

Checklist

  • I have read the CONTRIBUTING docs and have a developer setup (especially important are pre-commitand pytest)
  • Non-breaking change (would not break existing functionality)
  • Breaking change (would cause existing functionality to change)
  • Tests added or modified to cover the changes
  • Integration tests passed locally by running pytest
  • In-line docstrings updated
  • Documentation updated, tested running make html inside the docs/ folder
  • This pull request is ready to be reviewed
  • If the PR is ready and there are multiple commits, I have squashed them and force-pushed

@codecov
Copy link

codecov bot commented Feb 26, 2022

Codecov Report

Merging #832 (7c5493a) into main (eddd02d) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #832      +/-   ##
==========================================
- Coverage   93.87%   93.86%   -0.02%     
==========================================
  Files          72       72              
  Lines        4524     4514      -10     
==========================================
- Hits         4247     4237      -10     
  Misses        277      277              
Impacted Files Coverage Δ
torchio/data/inference/aggregator.py 97.43% <100.00%> (-0.30%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eddd02d...7c5493a. Read the comment docs.

@fepegar fepegar merged commit f5a2d74 into main Feb 26, 2022
@fepegar fepegar deleted the 813-fix-aggregator-overlap branch February 26, 2022 19:09
@fepegar
Copy link
Owner Author

fepegar commented Feb 26, 2022

Phew, this was a tough one. Thanks for reporting, @Hsuxu. May I add you to the list of contributors?

@Hsuxu
Copy link

Hsuxu commented Feb 28, 2022

Of course.

@fepegar
Copy link
Owner Author

fepegar commented Feb 28, 2022

@all-contributors please add @Hsuxu for bug.

@allcontributors
Copy link
Contributor

@fepegar

I've put up a pull request to add @Hsuxu! 🎉

justusschock added a commit to justusschock/torchio that referenced this pull request Jun 28, 2022
Use type of current instance instead of hardcoded subject type

fix copy behavior

Stop running some unstable tests on CI (fepegar#796)

* Print output messages during testing

* Skip MedMNIST tests on GitHub Actions

* Fix AttributeError

* Add reason for skipping test

Add support to specify the interpolation type for label images (fepegar#791)

* Add label_interpolation option in spatial transforms and augmentations

* Add label_interpolation option in spatial transforms and augmentations

* Adds missing backtick

* update docs 782

* Minor docs edits

Co-authored-by: Fernando Pérez-García <[email protected]>

docs: add snavalm as a contributor for code (fepegar#802)

* docs: update README.md [skip ci]

* docs: update .all-contributorsrc [skip ci]

Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>

Bump version: 0.18.71 → 0.18.72

Add py.typed marker file for type checking (fepegar#808)

docs: add jcreinhold as a contributor for code (fepegar#809)

* docs: update README.md [skip ci]

* docs: update .all-contributorsrc [skip ci]

Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>

Bump version: 0.18.72 → 0.18.73

Fix overlapping in patches aggregator (fepegar#832)

Bump version: 0.18.73 → 0.18.74

Update announcement

docs: add Hsuxu as a contributor for bug (fepegar#833)

* docs: update README.md [skip ci]

* docs: update .all-contributorsrc [skip ci]

Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>

Add check for shapes of input batch and output location

Add support for torch >= 1.11 (fepegar#838)

* added support for pytorch>=1.11.0

* Replace type with isinstance

Co-authored-by: Fernando Pérez-García <[email protected]>

Bump version: 0.18.74 → 0.18.75

docs: add snipdome as a contributor for bug (fepegar#839)

* docs: update README.md [skip ci]

* docs: update .all-contributorsrc [skip ci]

Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>

Remove redundant tests in GitHub actions (fepegar#850)

Fix new SimpleITK release breaking tests (fepegar#852)

Bump version: 0.18.75 → 0.18.76

Disable fail-fast in CI matrix strategy

Fix wrong mapping in SequentialLabels transform (fepegar#841)

docs: add iamSmallY as a contributor for bug (fepegar#855)

* docs: update README.md [skip ci]

* docs: update .all-contributorsrc [skip ci]

Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>

Fix SimpleITK version in docs requirements

Fix examples format in docstrings

Improve documentation and tests for label transforms (fepegar#857)

* Improve documentation and tests for label transforms

* Add parametrization with duplicated values in dict

Add example of RandomLabelsToImage

Add example of RandomGamma

Use sphinx-opengraph for documentation

Improve quality of figures from plot directive

[pre-commit.ci] pre-commit autoupdate (fepegar#859)

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

updates:
- [github.com/pre-commit/pre-commit-hooks: v4.0.1 → v4.2.0](pre-commit/pre-commit-hooks@v4.0.1...v4.2.0)
- [github.com/pycqa/flake8: 3.9.2 → 4.0.1](PyCQA/flake8@3.9.2...4.0.1)
- [github.com/asottile/pyupgrade: v2.29.0 → v2.32.0](asottile/pyupgrade@v2.29.0...v2.32.0)

* Fix pre-commit not installing flake8

This commit address the comments by @asottile in
pre-commit-ci/issues#118.

Thanks, @asottile!

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Fernando Pérez-García <[email protected]>

Validate arguments of add/remove_image

Return figure in plot_volume function (fepegar#872)

Co-authored-by: Derk <[email protected]>

docs: add guigautier as a contributor for ideas (fepegar#875)

* docs: update README.md [skip ci]

* docs: update .all-contributorsrc [skip ci]

Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>

Update flake8 error codes

[pre-commit.ci] pre-commit autoupdate (fepegar#874)

updates:
- [github.com/asottile/pyupgrade: v2.32.0 → v2.32.1](asottile/pyupgrade@v2.32.0...v2.32.1)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Fernando Pérez-García <[email protected]>

Stop upgrading pip in CI (fepegar#877)

Update bug report template

docs: add AyedSamy as a contributor for bug (fepegar#878)

* docs: update README.md [skip ci]

* docs: update .all-contributorsrc [skip ci]

Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>

Add tests for Python 3.10 (fepegar#880)

* Add tests for Python 3.10

* Fix Python version in YAML

Use PyTorch to compute Fourier transforms (fepegar#389)

* Use PyTorch for FFT if available

* Make sure tensor in on CPU before calling numpy()

Remove support for Python 3.6 (fepegar#881)

* Remove support for Python 3.6

* Fix Python version

Add support to use different numbers of samples in the queue (fepegar#795)

docs: add jmlipman as a contributor for ideas, code (fepegar#882)

* docs: update README.md [skip ci]

* docs: update .all-contributorsrc [skip ci]

Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>
Co-authored-by: Juan Miguel <[email protected]>

Fix computation of kernels in `Blur` transform (fepegar#861)

* Fix size of Gaussian kernel for blurring

* Fix typo in docstring

Add support to pass label keys for dict input (fepegar#879)

* Add support to pass label keys for dict input

* Add reference to @josegcpa's code

Co-authored-by: josegcpa <[email protected]>

docs: add josegcpa as a contributor for ideas (fepegar#883)

* docs: update README.md [skip ci]

* docs: update .all-contributorsrc [skip ci]

Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>

Bump version: 0.18.76 → 0.18.77

Fix support for NiBabel inputs

Remove unnecessary check

Remove error for multichannel NiBabel input

Cover method to print queue memory

Add gallery example for custom resampling transform

docs: add saikhu as a contributor for bug (fepegar#887)

* docs: update README.md [skip ci]

* docs: update .all-contributorsrc [skip ci]

Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>

Move package code to `src` directory (fepegar#896)

* Move package to src directory

* Update setup.py

* Update setup.py

* Move requirements

* Include requirements file in MANIFEST.in

* Stop using requirements file

* Fix warning in docs

Remove setup.py script and add pyproject.toml config file (fepegar#898)

* Remove setup.py and add pyproject.toml

* Add version to setup config

* Remove bumpversion config for setup.py

* Fix path to __init__

* Stop removing trailing whitespaces from setup.cfg

See peritus/bumpversion#78

* Update version replacing in bumpversion config

* Fix tox configuration for pyproject.toml

* Add keywords

* Remove requirements file

* Update authors config

Bump version: 0.18.77 → 0.18.78

Enable verbose mode for publishing GHA

Fix version

Fix version not being updated

As it seems that bump2version is not able to modify its own config file.

Add citation file

[pre-commit.ci] pre-commit autoupdate (fepegar#902)

updates:
- [github.com/pre-commit/pre-commit-hooks: v4.2.0 → v4.3.0](pre-commit/pre-commit-hooks@v4.2.0...v4.3.0)
- [github.com/asottile/pyupgrade: v2.32.1 → v2.34.0](asottile/pyupgrade@v2.32.1...v2.34.0)

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

Fix warning in RTD documentation build (fepegar#903)

Add hook for trailing commas (fepegar#904)

* Add hook for trailing commas

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

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

* Fix flake8 issues

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

Disable Binder buttons in Sphinx gallery (fepegar#905)

* Disable Binder buttons in Sphinx gallery

* Update RTD config to use pip installation

* Fix extra requirements string

* Add missing doc requirement

Add checks for TOML and YAML files

Add support to mask 4D images with 3D masks (fepegar#908)

* Add failing test

* Add support to mask 4D images with 3D masks

* Improve docstring

Co-authored-by: cbri92 <[email protected]>
Co-authored-by: valabregue <[email protected]>

docs: add cbri92 as a contributor for bug (fepegar#909)

* docs: update README.md [skip ci]

* docs: update .all-contributorsrc [skip ci]

Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>

Bump version: 0.18.78 → 0.18.79

Add fftshift to fourier_transform with torch.fft (fepegar#912)

This makes the behavior of fourier_transform and inv_fourier_transform
consistent with earlier versions of torchio. It is also independent of
whether torch or numpy is used to perform the transform.

Solves fepegar#911

docs: add iimog as a contributor for bug (fepegar#913)

* docs: update README.md [skip ci]

* docs: update .all-contributorsrc [skip ci]

Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>

Bump version: 0.18.79 → 0.18.80

Remove incorrect Contributors badge

Stop ignoring some missing imports for mypy

Fix tox running on Python 3.7 only (fepegar#915)

Create dependabot.yml (fepegar#917)

Test all Python versions in Ubuntu only (fepegar#918)

* Test all Python versions in Ubuntu only

* Fix exclusions

Bump actions/setup-python from 2 to 4 (fepegar#919)

Bumps [actions/setup-python](https://github.com/actions/setup-python) from 2 to 4.
- [Release notes](https://github.com/actions/setup-python/releases)
- [Commits](actions/setup-python@v2...v4)

---
updated-dependencies:
- dependency-name: actions/setup-python
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

Ignore some files in MANIFEST (fepegar#916)

* Ignore some files in MANIFEST

* Always publish to TestPyPI

* Skip existing TestPyPI versions

Bump actions/checkout from 2 to 3 (fepegar#920)

Bumps [actions/checkout](https://github.com/actions/checkout) from 2 to 3.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@v2...v3)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

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

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

Update fpg.py

Proper copy of subclass copies

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

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

fix parenthesis

imports

Fix proper subject subclass copy

old-style type annotations

linting

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

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

Fix proper subject subclass copy
@fepegar fepegar restored the 813-fix-aggregator-overlap branch July 4, 2022 22:00
@fepegar fepegar deleted the 813-fix-aggregator-overlap branch July 4, 2022 22:05
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.

GridAggregator result not match the input.
2 participants