Skip to content

Conversation

@rijobro
Copy link
Contributor

@rijobro rijobro commented Mar 17, 2021

Test time augmentations.

closes #718

Status

Ready

Types of changes

  • Non-breaking change (fix or new feature that would not break existing functionality).
  • New tests added to cover the changes.
  • Integration tests passed locally by running ./runtests.sh -f -u --net --coverage.
  • Quick tests passed locally by running ./runtests.sh --quick --unittests.
  • In-line docstrings updated.
  • Documentation updated, tested make html command in the docs/ folder.

rijobro added 2 commits March 4, 2021 15:50
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
@rijobro rijobro requested review from Nic-Ma, ericspod and wyli March 17, 2021 16:44
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
@rijobro
Copy link
Contributor Author

rijobro commented Mar 17, 2021

requires #1795.

@rijobro rijobro added the enhancement New feature or request label Mar 18, 2021
@rijobro rijobro mentioned this pull request Mar 19, 2021
6 tasks
batch_output = torch.Tensor(batch_output)

# check binary labels are extracted
if not all(torch.unique(batch_output.int()) == torch.Tensor([0, 1])):
Copy link
Member

Choose a reason for hiding this comment

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

This class will only work with segmentation networks then? Could it theoretically make sense for other tasks like image-to-image transformation? The returned values may make less sense perhaps.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've removed the check. We have the return_full_data if the user doesn't want to use the default metrics (e.g., computing the mode probably doesn't make sense for floating point numbers).

rijobro added 5 commits March 20, 2021 15:34
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
@rijobro
Copy link
Contributor Author

rijobro commented Mar 22, 2021

@wyli do you think you could provide some insight into this error? I can't see how any of the files I've modified could cause this.

Also I'm getting a flake error. This comes from:

    if isinstance(transform, Compose):
        # call recursively for each sub-transform
        return any(is_transform_rand_invertible(t) for t in transform.transforms)

When the if statement is satisfied, transform must therefore be of type Compose and therefore has the attribute .transforms. I thought flake was smart enough to figure this out? It does locally, at least.

rijobro added 2 commits March 22, 2021 11:18
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
@wyli
Copy link
Contributor

wyli commented Mar 22, 2021

Hi @rijobro for the first error, I think it's because monai.data is now depending on ignite, this is unexpected (the user should be able to use monai.data without installing ignite).

to replicate the issue in a system without ignite:

>>> import monai.data
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/opt/monai/monai/data/__init__.py", line 37, in <module>
    from .test_time_augmentation import TestTimeAugmentation
  File "/opt/monai/monai/data/test_time_augmentation.py", line 21, in <module>
    from monai.engines.utils import CommonKeys
  File "/opt/monai/monai/engines/__init__.py", line 12, in <module>
    from .evaluator import EnsembleEvaluator, Evaluator, SupervisedEvaluator
  File "/opt/monai/monai/engines/evaluator.py", line 17, in <module>
    from monai.engines.utils import CommonKeys as Keys
  File "/opt/monai/monai/engines/utils.py", line 33, in <module>
    class IterationEvents(EventEnum):
  File "/opt/monai/monai/utils/module.py", line 232, in __getattr__
    raise self._exception
  File "/opt/monai/monai/utils/module.py", line 191, in optional_import
    pkg = __import__(module)  # top level module
monai.utils.module.OptionalImportError: from ignite.engine import EventEnum (No module named 'ignite').

@wyli
Copy link
Contributor

wyli commented Mar 22, 2021

I couldn't replicate the pytype error either... perhaps have a # type: ignore?

Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
@rijobro
Copy link
Contributor Author

rijobro commented Mar 22, 2021

@wyli ok so the ignite must be because I'm importing CommonKeys which sits in engines. It should probably be moved to enums.py so that all modules can access it.

@wyli
Copy link
Contributor

wyli commented Mar 22, 2021

@wyli ok so the ignite must be because I'm importing CommonKeys which sits in engines. It should probably be moved to enums.py so that all modules can access it.

agreed, I think they are generic properties. cc @Nic-Ma this would be a breaking change (not a breaking change by importing the variables)

Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
@rijobro rijobro mentioned this pull request Mar 22, 2021
wyli and others added 3 commits March 22, 2021 20:05
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
@rijobro
Copy link
Contributor Author

rijobro commented Mar 23, 2021

Hi @wyli, current tests are failing: https://github.com/Project-MONAI/MONAI/pull/1794/checks?check_run_id=2169625488#step:11:5093. But device is set with device = "cuda" if torch.cuda.is_available() else "cpu". Any idea what's going on?

@wyli
Copy link
Contributor

wyli commented Mar 23, 2021

Hi @wyli, current tests are failing: https://github.com/Project-MONAI/MONAI/pull/1794/checks?check_run_id=2169625488#step:11:5093. But device is set with device = "cuda" if torch.cuda.is_available() else "cpu". Any idea what's going on?

strange, how do I get the corresponding code of that log?

@rijobro
Copy link
Contributor Author

rijobro commented Mar 23, 2021

Don't think you can do it from the logs, but you can obviously get it from the changed files in the PR. Here's the problematic test_testtimeaugmentations.py: https://github.com/Project-MONAI/MONAI/blob/e02a67ce4f94a81027d7006976aea2a5601e92d4/tests/test_testtimeaugmentation.py

@wyli
Copy link
Contributor

wyli commented Mar 23, 2021

Don't think you can do it from the logs, but you can obviously get it from the changed files in the PR. Here's the problematic test_testtimeaugmentations.py: https://github.com/Project-MONAI/MONAI/blob/e02a67ce4f94a81027d7006976aea2a5601e92d4/tests/test_testtimeaugmentation.py

the log corresponds to 6b488c https://github.com/Project-MONAI/MONAI/pull/1794/checks?check_run_id=2169625488#step:3:472 which is the commit before this device update, and the latest test doesn't show the error, correct?

I think the error is addressed by e02a67c e02a67c

rijobro added 4 commits March 23, 2021 09:49
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
Copy link
Contributor

@wyli wyli left a comment

Choose a reason for hiding this comment

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

thanks!

# check that whenever randoms is True, invertibles is also true
for r, i in zip(randoms, invertibles):
if r and not i:
raise RuntimeError(
Copy link
Contributor

@wyli wyli Mar 23, 2021

Choose a reason for hiding this comment

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

this could be a warning because the random factors may be in the model, we may want to do test aug with a randomised model...

rijobro added 2 commits March 23, 2021 14:08
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
@rijobro rijobro enabled auto-merge (squash) March 23, 2021 14:09
This reverts commit bb2e141.

Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
@rijobro rijobro merged commit 83855b9 into Project-MONAI:master Mar 23, 2021
@rijobro rijobro deleted the tta branch March 23, 2021 17:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Test-Time-Augmentation(TTA)

3 participants