-
Notifications
You must be signed in to change notification settings - Fork 107
Add Mayo Reader #1321
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
Add Mayo Reader #1321
Conversation
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.
Minor stuff
odl/contrib/datasets/ct/mayo.py
Outdated
|
||
Returns | ||
------- | ||
partition : odl.::: |
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.
Fix comment
|
||
if __name__ == '__main__': | ||
from odl.util.testutils import run_doctests | ||
run_doctests() |
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.
Missing newline
coords = [0, None, None] | ||
fbp_result.show('Recon (sagittal)', clim=[0.7, 1.3], coords=coords) | ||
ref.show('Reference (sagittal)', clim=[0.7, 1.3], coords=coords) | ||
(ref - fbp_result).show('Diff (sagittal)', clim=[-0.1, 0.1], coords=coords) |
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.
No newline
from odl.contrib.datasets.ct.mayo import (projections_from_folder, | ||
volume_from_folder) | ||
|
||
mayo_folder = 'E:/Data/MayoClinic' # replace with your local folder |
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.
How should we handle stuff like this? We cannot possibly provide the data due to licence issues.
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.
For the string: just a dot?
On the bigger point, I think this is the only sensible option.
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.
Ok, I'll change to that
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.
Not a whole lot to complain about, I'd say fix those and go ahead.
odl/contrib/datasets/README.md
Outdated
@@ -21,6 +21,11 @@ Reference datasets with accompanying ODL geometries etc. | |||
CT data as provided by FIPS. The data is non-human and high resolution. | |||
* `walnut_data` | |||
* `lotus_root_data` | |||
|
|||
|
|||
CT data as provided by Mayo Clinic. The data is human and high resolution. To access the data, see https://www.aapm.org/GrandChallenge/LowDoseCT/#registration |
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.
"The data is human" - errr what?
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.
Latest trend in big data
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.
Haha, yes that was my idea as well
"""Reconstruct Mayo dataset using FBP and compare to reference recon. | ||
|
||
Note that this example requires that Mayo has been previously downloaded and is | ||
stored in "mayo_folder". |
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.
I prefer the more correct term "directory" which can be abbreviated to the even shorter "dir". "folder" is so windows.
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.
Also, this may be misunderstood as literally a directory called "mayo_folder". Thus, mention that it's the variable with that name which counts.
odl/contrib/datasets/README.md
Outdated
@@ -21,6 +21,11 @@ Reference datasets with accompanying ODL geometries etc. | |||
CT data as provided by FIPS. The data is non-human and high resolution. | |||
* `walnut_data` | |||
* `lotus_root_data` | |||
|
|||
|
|||
CT data as provided by Mayo Clinic. The data is human and high resolution. To access the data, see https://www.aapm.org/GrandChallenge/LowDoseCT/#registration |
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.
Maybe you can also write a word or two on what you have to do to get the data (no details, just that it requires registration because it's potentially sensitive). I often find it frustrating to just be pointed to a website, only to find out 10 minutes later that I can't get it easily.
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.
I agree we should be explicit about that stuff
from odl.contrib.datasets.ct.mayo import (projections_from_folder, | ||
volume_from_folder) | ||
|
||
mayo_folder = 'E:/Data/MayoClinic' # replace with your local folder |
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.
For the string: just a dot?
On the bigger point, I think this is the only sensible option.
volume_folder = mayo_folder + '/Training Cases/L067/full_1mm_sharp' | ||
partition, volume = volume_from_folder(volume_folder) | ||
|
||
# Load projection data |
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.
a subset of the
odl/contrib/datasets/ct/mayo.py
Outdated
data_array = data_array.reshape([rows, cols], order='F').T | ||
|
||
# Rescale array | ||
# rescaled_array = rescale_intercept + data_array * rescale_slope |
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.
hu_factor
missing. But I think the code is pretty obvious, the comment is not really needed
odl/contrib/datasets/ct/mayo.py
Outdated
proj_start : int | ||
Index of the first projection to use. Used for subsampling. | ||
proj_end : int | ||
Index of the final projection to use. |
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.
This reads as inclusive, but the implementation is exclusive. Also the value -1 needs explanation.
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.
Comment was a leftover before I added the HU scaling
odl/contrib/datasets/ct/mayo.py
Outdated
geometry : ConeFlatGeometry | ||
Geometry corresponding to the Mayo projector. | ||
proj_data : `numpy.ndarray` | ||
Projection data, normalized such that the reconstruction is g/cm^3. |
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.
will have unit ...
Generally great to mention this!
odl/contrib/datasets/ct/mayo.py
Outdated
Parameters | ||
---------- | ||
folder : str | ||
Path to the folder where the dicom files are stored. |
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.
DICOM
odl/contrib/datasets/ct/mayo.py
Outdated
# Get data array and convert to correct coordinates | ||
data_array = np.array(np.frombuffer(dataset.PixelData, 'H'), | ||
dtype='float32') | ||
data_array = data_array.reshape([rows, cols], order='F').T |
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.
This is a complicated way of writing
data_array.reshape([cols, rows], order='C')
Will merge after CI |
Getting " |
I got the same in #1340. I thought it was due to the new documentation, (using r""" and removing double backslashes if I understood correctly), but I did not have time to investigate it further. However, you do not seem to have that, so maybe it is something else. |
I had a similar problem in #1267 (I think it was...) which resulted in a lot of commits where I removed the suppressing of the output from the build in Travis. I do not recall exactly what I did; after squashing it all became this commit. However, in the process I got a lot of help from @kohr-h |
See #1344 |
This adds a reader for Mayo clinic data.
Some examples:
It seems like the reconstruction is quite good as compared to the reference, so we should be "set".