Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

WIP: Add memory efficient meta data summary#1030

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
moloney wants to merge9 commits intonipy:master
base:master
Choose a base branch
Loading
frommoloney:enh-metasum

Conversation

@moloney
Copy link
Contributor

@moloneymoloney commentedJul 9, 2021
edited
Loading

This is some work-in-progress for adding data structures for creating a memory efficient summary of a sequence of meta data dictionaries (assuming a large number of keys/values repeat) and then using this to determine how to sort the associated images into an nD array.

This approach was inspired bythis dcmstack issue.

@pep8speaks
Copy link

pep8speaks commentedJul 9, 2021
edited
Loading

Hello@moloney, Thank you for updating!

Line 568:101:E501 line too long (102 > 100 characters)

To test for issues locally,pip install flake8 and then runflake8 nibabel.

Comment last updated at 2021-07-13 03:30:41 UTC

@codecov
Copy link

codecovbot commentedJul 9, 2021
edited
Loading

Codecov Report

Merging#1030 (bf8ecfc) intomaster (ea68c4e) willdecrease coverage by1.21%.
The diff coverage is58.96%.

Impacted file tree graph

@@            Coverage Diff             @@##           master    #1030      +/-   ##==========================================- Coverage   92.26%   91.04%   -1.22%==========================================  Files         100      101       +1       Lines       12205    12668     +463       Branches     2136     2267     +131     ==========================================+ Hits        11261    11534     +273- Misses        616      781     +165- Partials      328      353      +25
Impacted FilesCoverage Δ
nibabel/metasum.py58.96% <58.96%> (ø)

Continue to review full report at Codecov.

Legend -Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing data
Powered byCodecov. Last updateea68c4e...bf8ecfc. Read thecomment docs.

moloneyand others added2 commitsJuly 12, 2021 12:24
@effigies
Copy link
Member

Merged master in to resolve conflicts and get the tests going. Let me know if you'd prefer I didn't do that.

@ZviBaratz
Copy link
Contributor

@matthew-brett /@moloney /@effigies
If DICOM related functionality is moved to dicom_parser, do you still think theMetaSummary implementation will be required?
I feel like we could simply cache a dictionary of lazily evaluated header values within eachSeries instance. The higher levelDataset class (to be implemented) can simply query those.

@moloney
Copy link
ContributorAuthor

@ZviBaratz Can you explain in more detail what you have in mind? I don't see how a cache helps to solve the problem of determining what meta data is varying when someone hands us a list of Dicom files we have never seen before (that could come from multiple Dicom series).

@ZviBaratz
Copy link
Contributor

The idea is that there will be aDataset class which will receive a root directory and iterate its files to create the representations for the contained series. When a user tries to query based on any particular header field, the dataset queries all the createdSeries instances headers to retrieve the value (at which point it could be saved to a cache dictionary in order to avoid repeating computations). Of course some evaluation time is to be expected, but I don't think it should be anything too bad up to a few dozen series. If you're working with more than that, it might be best to export the metadata to some external table anyway.

@moloney
Copy link
ContributorAuthor

moloney commentedAug 9, 2021
edited
Loading

We really don't want to require all the files live in a single directory. The assumption is you are passed a list of files that could be massive even for a single series (e.g. 36K) that you have never seen before and you want to efficiently convert them into an xarray on the fly. My original implementation in dcmstack wasn't totally naive, meta data values that were constant were only stored once, and yet it required orders of magnitude more memory (18GB vs ~800MB with 36K files) compared to this approach.

@ZviBaratz
Copy link
Contributor

I see.
I'll be working on the issues that are already piling up in dicom_parser for the next couple of weeks, after that I'll start thinking on how this would best be integrated into dicom_parser. We could discuss it in more detail in our next meeting.

@moloney
Copy link
ContributorAuthor

If we want to support using multiprocessing to speed up the parsing of very large series, this would also provide a nice compact representation to pass around.

@effigies
Copy link
Member

Sorry, I lost track of this one. What's the status? Are we still trying to get this into nibabel?

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@effigieseffigieseffigies left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@moloney@pep8speaks@effigies@ZviBaratz

[8]ページ先頭

©2009-2025 Movatter.jp