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 support to use different numbers of samples in the queue #795

Merged
merged 2 commits into from
May 15, 2022

Conversation

fepegar
Copy link
Owner

@fepegar fepegar commented Jan 20, 2022

Related to:

Description
After these changes, one can decide how many patches to extract from each subject, using a num_samples kwarg when instantiating the subjects.

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 Jan 20, 2022

Codecov Report

Merging #795 (4118985) into main (cc9ccb3) will increase coverage by 0.09%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #795      +/-   ##
==========================================
+ Coverage   93.57%   93.67%   +0.09%     
==========================================
  Files          72       72              
  Lines        4562     4566       +4     
==========================================
+ Hits         4269     4277       +8     
+ Misses        293      289       -4     
Impacted Files Coverage Δ
torchio/constants.py 100.00% <100.00%> (ø)
torchio/data/queue.py 86.60% <100.00%> (+3.12%) ⬆️
torchio/data/dataset.py 100.00% <0.00%> (+1.96%) ⬆️

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@jmlipman
Copy link
Contributor

Hi Fernando.

I don't know whether I can download torchio and apply your pull request to try it locally, so, I could only read the code.

I like the idea of having a specific property to store the samples_per_volume (I was doing this, in practice). I have a question/concern. Consider the following scenario:

  • Queue's size is 50.
  • We are training on 2D slices of four 3D images.
  • Each image has 30 slices. (total slices 30*4=120).

I have the feeling that the queue will be filled like this:

First fill: 30 slices of image 1 + 20 slices of image 2.
Second fill: 30 slices of image 3 + 20 slices of image 4.
Consequently, ignoring 10 slices from image 2 and 10 from image 4. This is because every time "fill" is called it loads a new subject without checking if we are done taking the all the slices from the last subject the previous time "fill" was called. At least I don't see how this is done, but, as I've mentioned before, I didn't run the code and I might be wrong.

@fepegar
Copy link
Owner Author

fepegar commented Jan 30, 2022

I don't know whether I can download torchio and apply your pull request to try it locally, so, I could only read the code.

Maybe you can use

git clone -b 604-queue-different-samples [email protected]:fepegar/torchio.git

About the other concerns, I think that using a much larger queue length would make the stochastic process less sensitive to this issue. We might get fewer patches from a specific subject during an epoch, but that shouldn't happen in the next one if the subjects list is shuffled (which it is, by default). If you still think this is a concern, we can look into it and find alternatives.

@fepegar fepegar marked this pull request as draft January 30, 2022 15:06
@fepegar fepegar marked this pull request as ready for review May 15, 2022 22:03
@fepegar fepegar merged commit 8ee2ab2 into main May 15, 2022
@fepegar fepegar deleted the 604-queue-different-samples branch May 15, 2022 22:05
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:/actions/setup-python) from 2 to 4.
- [Release notes](https:/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:/actions/checkout) from 2 to 3.
- [Release notes](https:/actions/checkout/releases)
- [Changelog](https:/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 604-queue-different-samples branch July 4, 2022 22:01
@fepegar fepegar deleted the 604-queue-different-samples branch July 4, 2022 22:03
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