Skip to content

Conversation

@SachidanandAlle
Copy link
Contributor

@SachidanandAlle SachidanandAlle commented Dec 23, 2020

Signed-off-by: Sachidanand Alle salle@nvidia.com

Fixes # .

Description

Support Deepgrow 2D/3D training (training example will be added into Tutorials)
Earlier discussions captured in: #1329

Status

Ready/Work in progress/Hold

Types of changes

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

@wyli @Nic-Ma ^^

Signed-off-by: Sachidanand Alle <salle@nvidia.com>
SachidanandAlle and others added 9 commits December 23, 2020 12:46
Signed-off-by: Sachidanand Alle <salle@nvidia.com>
Signed-off-by: Sachidanand Alle <salle@nvidia.com>
Signed-off-by: Sachidanand Alle <salle@nvidia.com>
Signed-off-by: Sachidanand Alle <salle@nvidia.com>
Signed-off-by: Sachidanand Alle <salle@nvidia.com>
Signed-off-by: Sachidanand Alle <salle@nvidia.com>
Signed-off-by: Sachidanand Alle <salle@nvidia.com>
Signed-off-by: Sachidanand Alle <salle@nvidia.com>
@SachidanandAlle SachidanandAlle changed the title [WIP] Support to train/run Deepgrow 2D/3D models Support to train/run Deepgrow 2D/3D models Feb 8, 2021
SachidanandAlle and others added 9 commits February 8, 2021 13:15
Signed-off-by: Sachidanand Alle <salle@nvidia.com>
Signed-off-by: Sachidanand Alle <salle@nvidia.com>
Signed-off-by: Sachidanand Alle <salle@nvidia.com>
Signed-off-by: Yuan-Ting Hsieh <yuantingh@nvidia.com>
Signed-off-by: Sachidanand Alle <salle@nvidia.com>
Signed-off-by: Sachidanand Alle <salle@nvidia.com>
Signed-off-by: Sachidanand Alle <salle@nvidia.com>
Signed-off-by: Sachidanand Alle <salle@nvidia.com>
@SachidanandAlle
Copy link
Contributor Author

SachidanandAlle commented Feb 9, 2021

#1329 (comment)

  • the transforms should inherit monai.transforms.Transform - Fixed
  • ideally the core functionality should only rely on numpy and torch, the other packages should be imported with the optional_import method - Fixed
  • make_grid_with_titles looks nice, it could be in the visualize folder - Removed/Fixed
  • scipy.ndimage.filters.gaussian_filter could be replaced by GaussianFilter(nn.Module) - Wont Fix as performance gap was found in deepgrow model while using this approach.
  • DeepgrowStatsHandler implements quite a few features might - Removed/Fixed
  • some overview docs/links about how deepgrow works - TODO

@wyli ^^

Copy link
Contributor

@Nic-Ma Nic-Ma left a comment

Choose a reason for hiding this comment

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

Thanks for the quick update.
The PR overall looks good to me, put some comments inline.
And as it's very big, I hope @wyli or @ericspod , @rijobro also can help review it.

Thanks in advance.

.. autofunction:: download_and_extract

`Deepgrow`
-----------
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @YuanTingHsieh , please make the "-" length equals to Deepgrow.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed


def _default_transforms(keys, pixdim):
mode = [GridSampleMode.BILINEAR, GridSampleMode.NEAREST] if len(keys) == 2 else [GridSampleMode.BILINEAR]
transforms = [
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggest changing to:

return Compose([ ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

return data_list


def _save_data_3d(vol_idx, data, keys, dataset_dir, relative_path):
Copy link
Contributor

Choose a reason for hiding this comment

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

I think 2D and 3D have very similar logic, could you please merge them into 1 function?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

acutally its different.. 2D flattens slices over multiple labels and 3D only does for labels.
common things are already taken out of these 2 private functions.



def _save_data_2d(vol_idx, data, keys, dataset_dir, relative_path):
vol_image = data[keys[0]]
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe it's better to input image, label in args directly,


class Interaction:
"""
Deepgrow Training/Evaluation iteration method with interactions (simulation of clicks) support for image and label.
Copy link
Contributor

Choose a reason for hiding this comment

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

Please highlight in the doc-string that this class is an ignite handler.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

)
batchdata = self.transforms(batchdata)

return engine._iteration(engine, batchdata)
Copy link
Contributor

Choose a reason for hiding this comment

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

As this handler will attach to ITERATION_STARTED and the engine will run _iteration() later, why you call engine._iteration here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the batchdata is updated... and the _iteration() must happen on new modified batchdata. will returning batchdata is enough? and _iteration() in later call uses the updated batchdata? in that case, we need one round of training to make sure it works as expected.

this code was based on your initial feedback on how to run interactions while training. may be something got changed recently. nevertheless, you can confirm what to make _iteration run on new/updated batchdata in above case.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggest to use:

engine.state.batch = self.transforms(batchdata)

Then return nothing in this function.

Thanks.

Copy link
Contributor Author

@SachidanandAlle SachidanandAlle Feb 11, 2021

Choose a reason for hiding this comment

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

you mean engine.state.batch=batchdata
but it failed on post transform ( i guess engine.state.output is empty )

Traceback (most recent call last):
File "/opt/conda/lib/python3.6/site-packages/ignite/engine/engine.py", line 730, in _internal_run
time_taken = self._run_once_on_dataset()
File "/opt/conda/lib/python3.6/site-packages/ignite/engine/engine.py", line 828, in _run_once_on_dataset
self._handle_exception(e)
File "/opt/conda/lib/python3.6/site-packages/ignite/engine/engine.py", line 465, in _handle_exception
self._fire_event(Events.EXCEPTION_RAISED, e)
File "/opt/conda/lib/python3.6/site-packages/ignite/engine/engine.py", line 423, in _fire_event
func(*first, *(event_args + others), **kwargs)
File "/opt/monai/monai/handlers/stats_handler.py", line 145, in exception_raised
raise e
File "/opt/conda/lib/python3.6/site-packages/ignite/engine/engine.py", line 812, in _run_once_on_dataset
self._fire_event(Events.ITERATION_COMPLETED)
File "/opt/conda/lib/python3.6/site-packages/ignite/engine/engine.py", line 423, in _fire_event
func(*first, *(event_args + others), **kwargs)
File "/opt/monai/monai/engines/workflow.py", line 150, in run_post_transform
engine.state.output = apply_transform(posttrans, engine.state.output)
File "/opt/monai/monai/transforms/utils.py", line 387, in apply_transform
raise RuntimeError(f"applying transform {transform}") from e
RuntimeError: applying transform <monai.transforms.compose.Compose object at 0x7ff083933e80>
Traceback (most recent call last):
File "/opt/monai/monai/transforms/utils.py", line 385, in apply_transform
return transform(data)
File "/opt/monai/monai/transforms/post/dictionary.py", line 91, in call
d = dict(data)
TypeError: 'NoneType' object is not iterable

Traceback (most recent call last):
File "/opt/conda/lib/python3.6/runpy.py", line 193, in _run_module_as_main
"main", mod_spec)
File "/opt/conda/lib/python3.6/runpy.py", line 85, in _run_code
exec(code, run_globals)
File "apps/train.py", line 35, in
File "apps/train.py", line 27, in main
File "apps/mmar_conf.py", line 32, in train_mmar
File "/opt/monai/monai/engines/trainer.py", line 48, in run
super().run()
File "/opt/monai/monai/engines/workflow.py", line 191, in run
super().run(data=self.data_loader, max_epochs=self.state.max_epochs)
File "/opt/conda/lib/python3.6/site-packages/ignite/engine/engine.py", line 691, in run
return self._internal_run()
File "/opt/conda/lib/python3.6/site-packages/ignite/engine/engine.py", line 762, in _internal_run
self._handle_exception(e)
File "/opt/conda/lib/python3.6/site-packages/ignite/engine/engine.py", line 465, in _handle_exception
self._fire_event(Events.EXCEPTION_RAISED, e)
File "/opt/conda/lib/python3.6/site-packages/ignite/engine/engine.py", line 423, in _fire_event
func(*first, *(event_args + others), **kwargs)
File "/opt/monai/monai/handlers/stats_handler.py", line 145, in exception_raised
raise e
File "/opt/conda/lib/python3.6/site-packages/ignite/engine/engine.py", line 730, in _internal_run
time_taken = self._run_once_on_dataset()
File "/opt/conda/lib/python3.6/site-packages/ignite/engine/engine.py", line 828, in _run_once_on_dataset
self._handle_exception(e)
File "/opt/conda/lib/python3.6/site-packages/ignite/engine/engine.py", line 465, in _handle_exception
self._fire_event(Events.EXCEPTION_RAISED, e)
File "/opt/conda/lib/python3.6/site-packages/ignite/engine/engine.py", line 423, in _fire_event
func(*first, *(event_args + others), **kwargs)
File "/opt/monai/monai/handlers/stats_handler.py", line 145, in exception_raised
raise e
File "/opt/conda/lib/python3.6/site-packages/ignite/engine/engine.py", line 812, in _run_once_on_dataset
self._fire_event(Events.ITERATION_COMPLETED)
File "/opt/conda/lib/python3.6/site-packages/ignite/engine/engine.py", line 423, in _fire_event
func(*first, *(event_args + others), **kwargs)
File "/opt/monai/monai/engines/workflow.py", line 150, in run_post_transform
engine.state.output = apply_transform(posttrans, engine.state.output)
File "/opt/monai/monai/transforms/utils.py", line 387, in apply_transform
raise RuntimeError(f"applying transform {transform}") from e
RuntimeError: applying transform <monai.transforms.compose.Compose object at 0x7ff083933e80>


label = (label > 0.5).astype(np.float32)
blobs_labels = measure.label(label.astype(int), background=0) if dims == 2 else label
assert np.max(blobs_labels) > 0, "Not a valid Label"
Copy link
Contributor

Choose a reason for hiding this comment

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

suggest to use raise error.

return data


class SpatialCropForegroundd(MapTransform):
Copy link
Contributor

Choose a reason for hiding this comment

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

We already have CropForegroundd in MONAI: https://github.com/Project-MONAI/MONAI/blob/master/monai/transforms/croppad/dictionary.py#L367.
Can it satisfy your request here?

if not isinstance(keys, list) and not isinstance(keys, tuple):
keys = [keys]

transforms = _default_transforms(keys, pixdim) if transforms is None else transforms
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @wyli @SachidanandAlle , I feel this create_datalist() function actually can be a dict transform which works after these transforms, what do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

based on Daguang/research ask, we are providing only utility to flatten/pre-process the data prior to training.
otherwise, there can be very good chained dataset logic possible where some transforms contribute to create new set of dataset. keeping it simple based on the ask.

YuanTingHsieh and others added 4 commits February 10, 2021 18:04
Signed-off-by: Yuan-Ting Hsieh <yuantingh@nvidia.com>
Signed-off-by: Yuan-Ting Hsieh <yuantingh@nvidia.com>
Signed-off-by: Sachidanand Alle <salle@nvidia.com>
@SachidanandAlle
Copy link
Contributor Author

Marking this as WIP.. and will split and raise PRs with smaller changes

@SachidanandAlle SachidanandAlle changed the title Support to train/run Deepgrow 2D/3D models [WIP] Support to train/run Deepgrow 2D/3D models Feb 11, 2021
@wyli
Copy link
Contributor

wyli commented Feb 20, 2021

closing in favour of #1581 #1579 #1582

@wyli wyli closed this Feb 20, 2021
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.

4 participants