-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Add in_memory option to PersistentDataset for hybrid caching #8691
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
base: dev
Are you sure you want to change the base?
Changes from all commits
749a518
1d873cf
9b0166b
b572369
da09559
5e111f6
b5c3da1
17c64c7
352f0c5
152f7a0
efcfa46
3177ee1
876b799
247a1d8
e536444
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -236,6 +236,7 @@ def __init__( | |
| reset_ops_id: bool = True, | ||
| track_meta: bool = False, | ||
| weights_only: bool = True, | ||
| in_memory: bool = False, | ||
| ) -> None: | ||
| """ | ||
| Args: | ||
|
|
@@ -276,6 +277,14 @@ def __init__( | |
| Setting to `False` should only be done if it's absolutely necessary to load unsafe pickled data, | ||
| eg. MetaTensor objects with unsafe objects in their metadata. Users must verify the safety of the data | ||
| they intend to load before doing so. | ||
| in_memory: if `True`, also keep the pre-processed data in RAM after first access, so that later | ||
| epochs skip the disk cache entirely. This combines the benefits of persistent storage (data | ||
| survives restarts) with faster RAM access. Note the cache is unbounded, so the whole dataset | ||
| is eventually held in memory; use `CacheDataset` or `SmartCacheDataset` if that does not fit. | ||
| The RAM cache is per-process, so with `DataLoader(num_workers>0, persistent_workers=False)` | ||
| the cache is rebuilt every epoch; use `persistent_workers=True` (or `num_workers=0`) to retain | ||
| it across epochs, otherwise most of the benefit is lost. | ||
| Default to `False`. | ||
| """ | ||
| super().__init__(data=data, transform=transform) | ||
| self.cache_dir = Path(cache_dir) if cache_dir is not None else None | ||
|
|
@@ -293,6 +302,17 @@ def __init__( | |
| self.reset_ops_id = reset_ops_id | ||
| self.track_meta = track_meta | ||
| self.weights_only = weights_only | ||
| self.in_memory = in_memory | ||
| self._memory_cache: dict[int, Any] = {} | ||
|
|
||
| @property | ||
| def memory_cache_size(self) -> int: | ||
| """ | ||
| Returns: | ||
| The number of items currently stored in the in-memory cache. | ||
|
|
||
| """ | ||
| return len(self._memory_cache) | ||
|
|
||
| def set_transform_hash(self, hash_xform_func: Callable[..., bytes]): | ||
| """Get hashable transforms, and then hash them. Hashable transforms | ||
|
|
@@ -320,6 +340,7 @@ def set_data(self, data: Sequence): | |
|
|
||
| """ | ||
| self.data = data | ||
| self._memory_cache = {} | ||
| if self.cache_dir is not None and self.cache_dir.exists(): | ||
| shutil.rmtree(self.cache_dir, ignore_errors=True) | ||
| self.cache_dir.mkdir(parents=True, exist_ok=True) | ||
|
|
@@ -428,8 +449,24 @@ def _cachecheck(self, item_transformed): | |
| return _item_transformed | ||
|
|
||
| def _transform(self, index: int): | ||
| pre_random_item = self._cachecheck(self.data[index]) | ||
| return self._post_transform(pre_random_item) | ||
| """ | ||
| Fetch the pre-random-transform item for `index` and apply the random transforms to it. | ||
|
|
||
| Args: | ||
| index: index of the item in `self.data`. | ||
|
|
||
| Returns: | ||
| The fully transformed data element. | ||
|
|
||
| """ | ||
| if not self.in_memory: | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The RAM cache stores whatever _cachecheck() returns on first access, but that's the raw pre-transform value -- it never goes through the convert_to_tensor(..., track_meta=self.track_meta) normalization that happens before torch.save (around line 414). Without in_memory, every access after the first goes through the disk-hit branch of _cachecheck and gets that normalized (tensor/MetaTensor) type back via torch.load. With in_memory=True, the first computed value is cached verbatim and reused for every later access in that process, so it can permanently stay as e.g. a plain numpy.ndarray even when track_meta=True is set (which is documented elsewhere as "converts to MetaTensor"). I verified this with a minimal repro: a pre-random transform that leaves a field as numpy.ndarray returns numpy.ndarray on every access once in_memory=True, vs. numpy.ndarray then torch.Tensor (1st vs 2nd+ access) when in_memory=False. Suggested fix: when first populating self._memory_cache[index], run it through the same convert_to_tensor(..., track_meta=self.track_meta) call used before torch.save, so the RAM-cached entry matches what a disk-cache hit would return. |
||
| return self._post_transform(self._cachecheck(self.data[index])) | ||
| if index not in self._memory_cache: | ||
| self._memory_cache[index] = convert_to_tensor( | ||
| self._cachecheck(self.data[index]), convert_numeric=False, track_meta=self.track_meta | ||
| ) | ||
| # copy so that the random transforms, or the caller, cannot mutate the cached item | ||
| return self._post_transform(deepcopy(self._memory_cache[index])) | ||
|
|
||
|
|
||
| class CacheNTransDataset(PersistentDataset): | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_memory_cache is a plain per-process dict. With the common DataLoader(num_workers>0, persistent_workers=False) setup (the PyTorch default), worker processes are recreated every epoch, so the RAM cache is rebuilt from scratch each epoch and most of the "subsequent epochs skip disk reads" benefit is lost silently. This file already documents the equivalent caveat for CacheDataset/SmartCacheDataset (recommending persistent_workers=True); worth adding the same note here. Suggested change: add a line to the in_memory docstring noting that persistent_workers=True (or num_workers=0) is needed to actually retain the RAM cache across epochs.