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

update docstring and eval with overrides #6241

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 5 additions & 6 deletions monai/data/dataset.py
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,7 @@ def _pre_transform(self, item_transformed):

"""
if not isinstance(self.transform, Compose):
raise ValueError("transform must be an instance of monai.transforms.Compose.")
raise ValueError(f"transform must be an instance of monai.transforms.Compose, got {type(self.transform)}.")

first_random = self.transform.get_index_of_first(
lambda t: isinstance(t, RandomizableTrait) or not isinstance(t, Transform)
Expand Down Expand Up @@ -345,8 +345,7 @@ def _post_transform(self, item_transformed):
first_random = self.transform.get_index_of_first(
lambda t: isinstance(t, RandomizableTrait) or not isinstance(t, Transform)
)
if first_random is not None:
item_transformed = self.transform(item_transformed, start=first_random)
item_transformed = self.transform(item_transformed, start=first_random)
Copy link
Contributor

Choose a reason for hiding this comment

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

execute shouldn't have to handle start is None I think

return item_transformed

def _cachecheck(self, item_transformed):
Expand Down Expand Up @@ -489,7 +488,7 @@ def _pre_transform(self, item_transformed):
the transformed element up to the N transform object
"""
if not isinstance(self.transform, Compose):
raise ValueError("transform must be an instance of monai.transforms.Compose.")
raise ValueError(f"transform must be an instance of monai.transforms.Compose, got {type(self.transform)}.")

item_transformed = self.transform(item_transformed, end=self.cache_n_trans, threading=True)

Expand Down Expand Up @@ -906,14 +905,14 @@ def _transform(self, index: int):

# load data from cache and execute from the first random transform
if not isinstance(self.transform, Compose):
raise ValueError("transform must be an instance of monai.transforms.Compose.")
raise ValueError(f"transform must be an instance of monai.transforms.Compose, got {type(self.transform)}.")

first_random = self.transform.get_index_of_first(
lambda t: isinstance(t, RandomizableTrait) or not isinstance(t, Transform)
)
if first_random is not None:
data = deepcopy(data) if self.copy_cache is True else data
data = self.transform(data, start=first_random)
data = self.transform(data, start=first_random)

return data

Expand Down
76 changes: 54 additions & 22 deletions monai/transforms/compose.py
Original file line number Diff line number Diff line change
Expand Up @@ -262,8 +262,8 @@ def randomize(self, data: Any | None = None) -> None:
)

def get_index_of_first(self, predicate):
for i in range(len(self.transforms)):
if predicate(self.transforms[i]):
for i, xform in enumerate(self.transforms):
if predicate(xform):
return i
return None

Expand Down Expand Up @@ -294,11 +294,11 @@ def execute(
transforms: Sequence[Any],
map_items: bool = True,
unpack_items: bool = False,
start: int = 0,
start: int | None = 0,
end: int | None = None,
lazy_evaluation: bool = False,
overrides: dict = None,
override_keys: tuple = None,
overrides: dict | None = None,
override_keys: Sequence[str] | None = None,
threading: bool = False,
log_stats: bool = False,
verbose: bool = False,
Expand All @@ -309,27 +309,48 @@ def execute(
Compose and by code that doesn't have a Compose instance but needs to execute a
sequence of transforms is if it were executed by Compose. It should only be used directly
when it is not possible to use ``Compose.__call__`` to achieve the same goal.

Args:
`input_`: a tensor-like object to be transformed
input_: a tensor-like object to be transformed
transforms: a sequence of transforms to be carried out
map_items: whether to apply the transform to each item in ``data``.
Defaults to True if not set.
Defaults to True if not set.
unpack_items: whether to unpack parameters using '*'. Defaults to False if not set
log_stats: whether to log detailed information about the application of ``transforms``
to ``input_``. For NumPy ndarrays and PyTorch tensors, log only the data shape and
value range. Defaults to False if not set.
start: the index of the first transform to be executed. If not set, this defaults to 0
end: the index after the last transform to be exectued. If set, the transform at index-1
is the last transform that is executed. If this is not set, it defaults to len(transforms)
threading: whether executing is happening in a threaded environment. If set, copies are made
of transforms that have the ``RandomizedTrait`` interface.

Returns:

end: the index after the last transform to be exectued. If set, the transform at ``end-1``
is the last transform that is executed. If this is not set, it defaults to len(transforms)
lazy_evaluation: whether to enable lazy evaluation for lazy transforms. If False, transforms will be
carried out on a transform by transform basis. If True, all lazy transforms will
be executed by accumulating changes and resampling as few times as possible.
A `monai.transforms.Identity[D]` transform in the pipeline will trigger the evaluation of
the pending operations and make the primary data up-to-date.
overrides: this optional parameter allows you to specify a dictionary of parameters that should be overridden
when executing a pipeline. These each parameter that is compatible with a given transform is then applied
to that transform before it is executed. Note that overrides are currently only applied when lazy_evaluation
is True. If lazy_evaluation is False they are ignored.
currently supported args are:
{``"mode"``, ``"padding_mode"``, ``"dtype"``, ``"align_corners"``, ``"resample_mode"``, ``device``},
please see also :py:func:`monai.transforms.lazy.apply_transforms` for more details.
override_keys: this optional parameter specifies the keys to which ``overrides`` are to be applied. If
``overrides`` is set, ``override_keys`` must also be set.
threading: whether executing is happening in a threaded environment. If set, deepcopies are made
of transforms that have the ``RandomizedTrait`` interface.
log_stats: whether to log detailed information about the application of ``transforms``
to ``input_``. For NumPy ndarrays and PyTorch tensors, log only the data shape and
value range. Defaults to False if not set.
verbose: whether to print debugging info when lazy_evaluation=True.
"""
end_ = len(transforms) if end is None else end
if start is None:
raise ValueError(f"'start' ({start}) cannot be None")
input_ = evaluate_with_overrides(
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure this is the correct way to handle this issue. I know you are trying to isolate dataset from changes, but I don't understand why evaluate_with_overrides needs to be called

Copy link
Contributor

Choose a reason for hiding this comment

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

In any case, please hold off on this for now until I have assembled the PR that refactors some of this code for simplicity.

Copy link
Contributor Author

@wyli wyli Mar 28, 2023

Choose a reason for hiding this comment

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

mainly to replicate this line on the dev branch:

item_transformed = self.transform.evaluate_with_overrides(item_transformed, None)

so that the final transform always gets the pending operations evaluated

#6224 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

If first_random is None, that means your entire compose was run before the item was cached, which would include the final evaluation. When the item is loaded from the cache, it has no pending transforms, so you have no way for pending transforms to be added that would then need to be evaluated

input_,
None,
lazy_evaluation=lazy_evaluation,
overrides=overrides,
override_keys=override_keys,
verbose=verbose,
)
return input_
if start > end_:
raise ValueError(f"'start' ({start}) must be less than 'end' ({end_})")
if end_ > len(transforms):
Expand All @@ -350,7 +371,7 @@ def execute(
override_keys=override_keys,
verbose=verbose,
)
input_ = apply_transform(_transform, input_, map_items, unpack_items, log_stats)
input_ = apply_transform(_transform, input_, map_items, unpack_items, log_stats) # type: ignore
input_ = evaluate_with_overrides(
input_,
None,
Expand All @@ -370,21 +391,32 @@ def evaluate_with_overrides(self, input_, upcoming_xform):
return evaluate_with_overrides(
input_,
upcoming_xform,
lazy_evaluation=self.lazy_evaluation,
lazy_evaluation=bool(self.lazy_evaluation),
overrides=self.overrides,
override_keys=self.override_keys,
verbose=self.verbose,
)

def __call__(self, input_, start=0, end=None, threading=False):
def __call__(self, input_, start: int | None = 0, end: int | None = None, threading: bool = False):
"""
apply the ``self.transforms`` to ``input_``.

Args:
input_: input data to be transformed.
start: the index of the first transform to be executed. If not set, this defaults to 0.
end: the index after the last transform to be exectued. If set, the transform at ``end-1``
is the last transform that is executed. If this is not set, it defaults to len(transforms).
threading: whether executing is happening in a threaded environment. If set, deepcopies are made
of transforms that have the ``RandomizedTrait`` interface.
"""
return Compose.execute(
input_,
self.transforms,
start=start,
end=end,
map_items=self.map_items,
unpack_items=self.unpack_items,
lazy_evaluation=self.lazy_evaluation,
lazy_evaluation=bool(self.lazy_evaluation),
overrides=self.overrides,
override_keys=self.override_keys,
threading=threading,
Expand Down