Skip to content

update lmdbdataset#2531

Merged
wyli merged 6 commits into
Project-MONAI:devfrom
wyli:lmdb-improvements
Jul 8, 2021
Merged

update lmdbdataset#2531
wyli merged 6 commits into
Project-MONAI:devfrom
wyli:lmdb-improvements

Conversation

@wyli

@wyli wyli commented Jul 6, 2021

Copy link
Copy Markdown
Contributor

Signed-off-by: Wenqi Li wenqil@nvidia.com

this PR moves the lmdb _fill_cache_start_reader to the dataset's constructor, otherwise multiprocess lmdb writing may slow down the loading a lot

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.

@wyli wyli requested review from Nic-Ma, ericspod and rijobro July 6, 2021 15:34
@wyli

wyli commented Jul 6, 2021

Copy link
Copy Markdown
Contributor Author

/build

2 similar comments
@wyli

wyli commented Jul 6, 2021

Copy link
Copy Markdown
Contributor Author

/build

@madil90

madil90 commented Jul 7, 2021

Copy link
Copy Markdown
Contributor

/build

@Nic-Ma

Nic-Ma commented Jul 7, 2021

Copy link
Copy Markdown
Contributor

Hi @wyli and @madil90 ,

May I know why you guys put several comments /build here? Is this PR ready for review now?

Thanks.

@wyli

wyli commented Jul 7, 2021

Copy link
Copy Markdown
Contributor Author

Hi @wyli and @madil90 ,

May I know why you guys put several comments /build here? Is this PR ready for review now?

Thanks.

sorry, we were testing the premerge, they are irrelevant. this is ready for review, I tested it in a training tutorial, on both NGC and google colab.

Comment thread monai/data/dataset.py

@Nic-Ma Nic-Ma left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sure, could you please help add the single-writer & multi-reader feature of LMDB to the doc-string? Just in case users may ask the same question when the initial caching is very slow.
Others look good to me.

Thanks.

wyli added 5 commits July 8, 2021 11:06
Signed-off-by: Wenqi Li <wenqil@nvidia.com>
Signed-off-by: Wenqi Li <wenqil@nvidia.com>
Signed-off-by: Wenqi Li <wenqil@nvidia.com>
Signed-off-by: Wenqi Li <wenqil@nvidia.com>
Signed-off-by: Wenqi Li <wenqil@nvidia.com>
@wyli wyli force-pushed the lmdb-improvements branch from 00a873a to 9d6dbc3 Compare July 8, 2021 10:06
@wyli wyli enabled auto-merge (squash) July 8, 2021 11:58
@wyli wyli merged commit a980ae4 into Project-MONAI:dev Jul 8, 2021
wyli added a commit to wyli/MONAI that referenced this pull request Jul 8, 2021
This reverts commit a980ae4.

Signed-off-by: Wenqi Li <wenqil@nvidia.com>
@wyli wyli mentioned this pull request Jul 8, 2021
7 tasks
@wyli wyli deleted the lmdb-improvements branch July 8, 2021 17:10
wyli added a commit that referenced this pull request Jul 8, 2021
* adds whats new 0.6

Signed-off-by: Wenqi Li <wenqil@nvidia.com>

* update docs

Signed-off-by: Wenqi Li <wenqil@nvidia.com>

* add a list

Signed-off-by: Wenqi Li <wenqil@nvidia.com>

* update according to comments

Signed-off-by: Wenqi Li <wenqil@nvidia.com>

* fixes typos

Signed-off-by: Wenqi Li <wenqil@nvidia.com>

* Revert "update lmdbdataset (#2531)"

This reverts commit a980ae4.

Signed-off-by: Wenqi Li <wenqil@nvidia.com>
wyli added a commit to wyli/MONAI that referenced this pull request Jul 12, 2021
* update lmdbdataset

Signed-off-by: Wenqi Li <wenqil@nvidia.com>

* update tests

Signed-off-by: Wenqi Li <wenqil@nvidia.com>

* docstring and less verbose progressbar

Signed-off-by: Wenqi Li <wenqil@nvidia.com>

* docstring

Signed-off-by: Wenqi Li <wenqil@nvidia.com>

* update based on comments

Signed-off-by: Wenqi Li <wenqil@nvidia.com>
@wyli wyli mentioned this pull request Jul 12, 2021
7 tasks
wyli added a commit to wyli/MONAI that referenced this pull request Jul 13, 2021
* update lmdbdataset

Signed-off-by: Wenqi Li <wenqil@nvidia.com>

* update tests

Signed-off-by: Wenqi Li <wenqil@nvidia.com>

* docstring and less verbose progressbar

Signed-off-by: Wenqi Li <wenqil@nvidia.com>

* docstring

Signed-off-by: Wenqi Li <wenqil@nvidia.com>

* update based on comments

Signed-off-by: Wenqi Li <wenqil@nvidia.com>
Nic-Ma pushed a commit that referenced this pull request Jul 13, 2021
* update lmdbdataset (#2531)

* update lmdbdataset

Signed-off-by: Wenqi Li <wenqil@nvidia.com>

* update tests

Signed-off-by: Wenqi Li <wenqil@nvidia.com>

* docstring and less verbose progressbar

Signed-off-by: Wenqi Li <wenqil@nvidia.com>

* docstring

Signed-off-by: Wenqi Li <wenqil@nvidia.com>

* update based on comments

Signed-off-by: Wenqi Li <wenqil@nvidia.com>

* fixes lmdb integration tests

Signed-off-by: Wenqi Li <wenqil@nvidia.com>

* add docstring

Signed-off-by: Wenqi Li <wenqil@nvidia.com>
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.

3 participants