Skip to content

update lmdbdataset#2531

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

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

Conversation

@wyli
Copy link
Copy Markdown
Contributor

@wyli wyli commented Jul 6, 2021

Signed-off-by: Wenqi Li [email protected]

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
Copy link
Copy Markdown
Contributor Author

wyli commented Jul 6, 2021

/build

2 similar comments
@wyli
Copy link
Copy Markdown
Contributor Author

wyli commented Jul 6, 2021

/build

@madil90
Copy link
Copy Markdown
Contributor

madil90 commented Jul 7, 2021

/build

@Nic-Ma
Copy link
Copy Markdown
Contributor

Nic-Ma commented Jul 7, 2021

Hi @wyli and @madil90 ,

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

Thanks.

@wyli
Copy link
Copy Markdown
Contributor Author

wyli commented Jul 7, 2021

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.

Copy link
Copy Markdown
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.

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 <[email protected]>
Signed-off-by: Wenqi Li <[email protected]>
Signed-off-by: Wenqi Li <[email protected]>
@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 <[email protected]>
@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 <[email protected]>

* update docs

Signed-off-by: Wenqi Li <[email protected]>

* add a list

Signed-off-by: Wenqi Li <[email protected]>

* update according to comments

Signed-off-by: Wenqi Li <[email protected]>

* fixes typos

Signed-off-by: Wenqi Li <[email protected]>

* Revert "update lmdbdataset (#2531)"

This reverts commit a980ae4.

Signed-off-by: Wenqi Li <[email protected]>
wyli added a commit to wyli/MONAI that referenced this pull request Jul 12, 2021
* update lmdbdataset

Signed-off-by: Wenqi Li <[email protected]>

* update tests

Signed-off-by: Wenqi Li <[email protected]>

* docstring and less verbose progressbar

Signed-off-by: Wenqi Li <[email protected]>

* docstring

Signed-off-by: Wenqi Li <[email protected]>

* update based on comments

Signed-off-by: Wenqi Li <[email protected]>
@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 <[email protected]>

* update tests

Signed-off-by: Wenqi Li <[email protected]>

* docstring and less verbose progressbar

Signed-off-by: Wenqi Li <[email protected]>

* docstring

Signed-off-by: Wenqi Li <[email protected]>

* update based on comments

Signed-off-by: Wenqi Li <[email protected]>
Nic-Ma pushed a commit that referenced this pull request Jul 13, 2021
* update lmdbdataset (#2531)

* update lmdbdataset

Signed-off-by: Wenqi Li <[email protected]>

* update tests

Signed-off-by: Wenqi Li <[email protected]>

* docstring and less verbose progressbar

Signed-off-by: Wenqi Li <[email protected]>

* docstring

Signed-off-by: Wenqi Li <[email protected]>

* update based on comments

Signed-off-by: Wenqi Li <[email protected]>

* fixes lmdb integration tests

Signed-off-by: Wenqi Li <[email protected]>

* add docstring

Signed-off-by: Wenqi Li <[email protected]>
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