-
Notifications
You must be signed in to change notification settings - Fork 781
Add k-space basics tutorial for MRI reconstruction #2056
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: main
Are you sure you want to change the base?
Conversation
git push -u origin add-kspace-basics-tutorial Adds an introductory notebook covering k-space fundamentals, Fourier transform connection, aliasing from undersampling, and MONAI's reconstruction transforms using the fastMRI knee single-coil dataset. Also adds the missing Reconstruction section to the main README. Signed-off-by: Vidya Sagar <vidyasagarreddy.venna@gmail.com>
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
WalkthroughAdds a new MRI Reconstruction tutorial suite: a k-space fundamentals Jupyter notebook, a tutorials README, updates the project README to include a Reconstruction section, and registers the notebook in the runner exemption list. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
for more information, see https://pre-commit.ci
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.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@README.md`:
- Line 397: Remove the garbled terminal/encoding artifact "ECHO�� �����Ǿ�
�ֽ��ϴ�." found at the end of README.md (the text shown in the diff) so the file
ends cleanly; ensure you delete that line, trim any trailing blank lines or
whitespace, and save the README.md with UTF-8 encoding to prevent reintroducing
encoding artifacts.
In
`@reconstruction/MRI_reconstruction/tutorials/01_kspace_basics_fastmri_knee.ipynb`:
- Around line 150-159: Check that data_path exists and contains .h5 files before
using os.listdir and indexing sample_files: verify os.path.exists(data_path) and
os.path.isdir(data_path), catch FileNotFoundError from os.listdir (or test
existence first), build sample_files = [...] and then if not sample_files raise
a clear RuntimeError or ValueError explaining the dataset is missing and how to
download it, otherwise set sample_file = os.path.join(data_path,
sample_files[0]) and print as before; reference symbols: data_path, os.listdir,
sample_files, sample_file.
🧹 Nitpick comments (2)
reconstruction/MRI_reconstruction/tutorials/01_kspace_basics_fastmri_knee.ipynb (2)
462-466: Duplicate utility functions:as_numpyandto_numpy.
as_numpy(line 462) andto_numpy(line 561) are functionally identical — both convert a torch tensor or array to numpy. Consider defining one and reusing it.Also applies to: 561-565
47-51: Minor: spaces in pip extras specifier.
"monai-weekly[pillow, tqdm]"has a space after the comma in the extras specifier. While modern pip handles this, it's unconventional and could cause issues with older pip versions. Consider"monai-weekly[pillow,tqdm]".
| Training and inference for accelerated MRI reconstruction using BasicUNet on the fastMRI brain multi-coil dataset. | ||
| ##### [VarNet MRI Reconstruction](./reconstruction/MRI_reconstruction/varnet_demo) | ||
| Training and inference for accelerated MRI reconstruction using e2e-VarNet on the fastMRI brain multi-coil dataset. | ||
| ECHO�� �����Ǿ� �ֽ��ϴ�. |
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.
Remove garbled text at the end of the file.
Line 397 contains corrupted/non-English characters (ECHO�� �����Ǿ� �ֽ��ϴ�.) that appear to be a terminal artifact or encoding issue. This should be removed.
Proposed fix
-ECHO�� �����Ǿ� �ֽ��ϴ�.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ECHO�� �����Ǿ� �ֽ��ϴ�. |
🤖 Prompt for AI Agents
In `@README.md` at line 397, Remove the garbled terminal/encoding artifact "ECHO��
�����Ǿ� �ֽ��ϴ�." found at the end of README.md (the text shown in the diff) so
the file ends cleanly; ensure you delete that line, trim any trailing blank
lines or whitespace, and save the README.md with UTF-8 encoding to prevent
reintroducing encoding artifacts.
| "# Update this path to where your fastMRI knee single-coil data is stored.\n", | ||
| "# You only need ONE .h5 file from the knee_singlecoil_val set.\n", | ||
| "data_path = os.path.join(root_dir, \"knee_singlecoil_val\")\n", | ||
| "\n", | ||
| "sample_files = sorted(\n", | ||
| " [f for f in os.listdir(data_path) if f.endswith(\".h5\")]\n", | ||
| ")\n", | ||
| "sample_file = os.path.join(data_path, sample_files[0])\n", | ||
| "print(f\"Using sample file: {sample_file}\")\n", | ||
| "print(f\"Total .h5 files found: {len(sample_files)}\")" |
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 error handling when no .h5 files are found.
If data_path doesn't exist or contains no .h5 files, os.listdir will raise FileNotFoundError or sample_files[0] will raise IndexError with no helpful message. Since this notebook requires manual data download, a clear error message would improve the user experience.
Proposed improvement
"data_path = os.path.join(root_dir, \"knee_singlecoil_val\")\n",
"\n",
+"if not os.path.isdir(data_path):\n",
+" raise FileNotFoundError(\n",
+" f\"Data directory not found: {data_path}\\n\"\n",
+" \"Please download the fastMRI knee single-coil validation set \"\n",
+" \"from https://fastmri.org/dataset and extract it to the path above.\"\n",
+" )\n",
+"\n",
"sample_files = sorted(\n",
" [f for f in os.listdir(data_path) if f.endswith(\".h5\")]\n",
")\n",
+"if not sample_files:\n",
+" raise FileNotFoundError(\n",
+" f\"No .h5 files found in {data_path}. \"\n",
+" \"Please ensure the fastMRI data is extracted correctly.\"\n",
+" )\n",
+"\n",
"sample_file = os.path.join(data_path, sample_files[0])\n",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "# Update this path to where your fastMRI knee single-coil data is stored.\n", | |
| "# You only need ONE .h5 file from the knee_singlecoil_val set.\n", | |
| "data_path = os.path.join(root_dir, \"knee_singlecoil_val\")\n", | |
| "\n", | |
| "sample_files = sorted(\n", | |
| " [f for f in os.listdir(data_path) if f.endswith(\".h5\")]\n", | |
| ")\n", | |
| "sample_file = os.path.join(data_path, sample_files[0])\n", | |
| "print(f\"Using sample file: {sample_file}\")\n", | |
| "print(f\"Total .h5 files found: {len(sample_files)}\")" | |
| # Update this path to where your fastMRI knee single-coil data is stored. | |
| # You only need ONE .h5 file from the knee_singlecoil_val set. | |
| data_path = os.path.join(root_dir, "knee_singlecoil_val") | |
| if not os.path.isdir(data_path): | |
| raise FileNotFoundError( | |
| f"Data directory not found: {data_path}\n" | |
| "Please download the fastMRI knee single-coil validation set " | |
| "from https://fastmri.org/dataset and extract it to the path above." | |
| ) | |
| sample_files = sorted( | |
| [f for f in os.listdir(data_path) if f.endswith(".h5")] | |
| ) | |
| if not sample_files: | |
| raise FileNotFoundError( | |
| f"No .h5 files found in {data_path}. " | |
| "Please ensure the fastMRI data is extracted correctly." | |
| ) | |
| sample_file = os.path.join(data_path, sample_files[0]) | |
| print(f"Using sample file: {sample_file}") | |
| print(f"Total .h5 files found: {len(sample_files)}") |
🤖 Prompt for AI Agents
In
`@reconstruction/MRI_reconstruction/tutorials/01_kspace_basics_fastmri_knee.ipynb`
around lines 150 - 159, Check that data_path exists and contains .h5 files
before using os.listdir and indexing sample_files: verify
os.path.exists(data_path) and os.path.isdir(data_path), catch FileNotFoundError
from os.listdir (or test existence first), build sample_files = [...] and then
if not sample_files raise a clear RuntimeError or ValueError explaining the
dataset is missing and how to download it, otherwise set sample_file =
os.path.join(data_path, sample_files[0]) and print as before; reference symbols:
data_path, os.listdir, sample_files, sample_file.
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.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@reconstruction/MRI_reconstruction/tutorials/01_kspace_basics_fastmri_knee.ipynb`:
- Around line 753-755: The cleanup currently removes root_dir even when a temp
directory was created because MONAI_DATA_DIRECTORY is unset; instead, fail early
when MONAI_DATA_DIRECTORY is not provided so users know to set it (or explicitly
opt into using a temp dir). Update the notebook logic around the
directory/root_dir creation (the variable directory and root_dir) to check the
MONAI_DATA_DIRECTORY env var and raise a clear error if it's None (with guidance
to set MONAI_DATA_DIRECTORY to the fastMRI data path), and remove the
unconditional shutil.rmtree(root_dir) cleanup for the fallback case; ensure the
error is raised before any downstream operations (the cells that previously
error out) so the user gets a clear prompt rather than the notebook silently
creating and later deleting a useless temp dir.
- Around line 456-478: The MONAI pipeline returns a full volume but the code
uses ground_truth[slice_idx] (single slice); to fix, extract the same slice_idx
from the MONAI outputs before squeezing: when building rand_recon, index
random_result["kspace_masked_ifft"] with [slice_idx] (or appropriate
channel/slice axis) then call as_numpy(...).squeeze() and take abs; do the same
for equi_recon (and any other recon variables) so their shapes match
ground_truth and imshow renders the correct middle slice; reference variables:
as_numpy, random_result["kspace_masked_ifft"], rand_recon, equi_recon,
ground_truth, slice_idx.
🧹 Nitpick comments (2)
reconstruction/MRI_reconstruction/tutorials/01_kspace_basics_fastmri_knee.ipynb (2)
549-553: Duplicate helper:to_numpyis identical toas_numpy(line 456).Three near-identical tensor-to-numpy helpers are defined across the notebook (
as_numpy,to_numpy,as_numpy_2d). Consider defining a single utility once (e.g., in the imports cell) and reusing it, adding the 2D squeeze variant as an optional parameter or a second thin wrapper.
212-212: Non-standard FFT shift ordering (works for even dimensions only).The standard convention for MRI reconstruction is
fftshift(ifft2(ifftshift(kspace))), but this line usesifftshift(ifft2(fftshift(kspace)))— the shifts are swapped. For even-length dimensions (typical for fastMRI data),fftshiftandifftshiftare equivalent, so results are correct. However, since this is a pedagogical tutorial on k-space fundamentals, using the standard ordering would avoid confusing learners who cross-reference with textbooks or other MRI reconstruction code.Suggested fix
-image_from_kspace = np.fft.ifftshift(np.fft.ifft2(np.fft.fftshift(kspace_slice))) +image_from_kspace = np.fft.fftshift(np.fft.ifft2(np.fft.ifftshift(kspace_slice)))
| "def as_numpy(x):\n", | ||
| " \"\"\"Convert torch tensor or array to numpy.\"\"\"\n", | ||
| " if isinstance(x, torch.Tensor):\n", | ||
| " return x.detach().cpu().numpy()\n", | ||
| " return np.asarray(x)\n", | ||
| "\n", | ||
| "\n", | ||
| "fig, axes = plt.subplots(2, 3, figsize=(18, 12))\n", | ||
| "\n", | ||
| "# Row 1: Random mask\n", | ||
| "rand_mask = as_numpy(random_result[\"mask\"]).squeeze()\n", | ||
| "axes[0, 0].imshow(np.tile(rand_mask, (64, 1)), cmap=\"gray\", aspect=\"auto\")\n", | ||
| "axes[0, 0].set_title(\"Random mask pattern\")\n", | ||
| "axes[0, 0].axis(\"off\")\n", | ||
| "\n", | ||
| "rand_recon = np.abs(as_numpy(random_result[\"kspace_masked_ifft\"]).squeeze())\n", | ||
| "axes[0, 1].imshow(rand_recon, cmap=\"gray\")\n", | ||
| "axes[0, 1].set_title(\"Zero-filled recon (random 4x)\")\n", | ||
| "axes[0, 1].axis(\"off\")\n", | ||
| "\n", | ||
| "gt = np.abs(as_numpy(ground_truth[slice_idx]))\n", | ||
| "axes[0, 2].imshow(gt, cmap=\"gray\")\n", | ||
| "axes[0, 2].set_title(\"Ground truth\")\n", |
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.
Potential slice mismatch between MONAI output and ground truth.
In Part 4, the MONAI pipeline (lines 401-441) processes the entire volume (all slices), but the ground truth at line 476 uses ground_truth[slice_idx] which is the middle slice loaded earlier via raw h5py. The .squeeze() on line 471 won't isolate the matching middle slice from the multi-slice kspace_masked_ifft output — it only removes dimensions of size 1.
If the volume has >1 slice, rand_recon will still be 3D after squeeze, causing imshow to fail or render incorrectly, and the comparison with gt will be invalid.
Consider selecting the middle slice from the MONAI output to match the ground truth:
Proposed fix
-rand_recon = np.abs(as_numpy(random_result["kspace_masked_ifft"]).squeeze())
+rand_ifft = as_numpy(random_result["kspace_masked_ifft"])
+# Select the same middle slice used for ground truth
+if rand_ifft.ndim >= 3:
+ rand_ifft = rand_ifft[rand_ifft.shape[0] // 2]
+rand_recon = np.abs(rand_ifft.squeeze())Apply the same fix for equi_recon on line 487.
🤖 Prompt for AI Agents
In
`@reconstruction/MRI_reconstruction/tutorials/01_kspace_basics_fastmri_knee.ipynb`
around lines 456 - 478, The MONAI pipeline returns a full volume but the code
uses ground_truth[slice_idx] (single slice); to fix, extract the same slice_idx
from the MONAI outputs before squeezing: when building rand_recon, index
random_result["kspace_masked_ifft"] with [slice_idx] (or appropriate
channel/slice axis) then call as_numpy(...).squeeze() and take abs; do the same
for equi_recon (and any other recon variables) so their shapes match
ground_truth and imshow renders the correct middle slice; reference variables:
as_numpy, random_result["kspace_masked_ifft"], rand_recon, equi_recon,
ground_truth, slice_idx.
| "if directory is None:\n", | ||
| " shutil.rmtree(root_dir)\n", | ||
| "print(\"Cleanup complete!\")" |
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.
Cleanup will delete the temp directory even though it never contained user data.
When MONAI_DATA_DIRECTORY is not set, the notebook creates a temp dir but the user's fastMRI data would need to already be inside it for the notebook to work (it won't be). This means the notebook will fail much earlier at lines 154-155. The cleanup is harmless but the temp-dir fallback path is effectively unusable for this tutorial. Consider documenting this more explicitly or raising an error early when the env var isn't set.
🤖 Prompt for AI Agents
In
`@reconstruction/MRI_reconstruction/tutorials/01_kspace_basics_fastmri_knee.ipynb`
around lines 753 - 755, The cleanup currently removes root_dir even when a temp
directory was created because MONAI_DATA_DIRECTORY is unset; instead, fail early
when MONAI_DATA_DIRECTORY is not provided so users know to set it (or explicitly
opt into using a temp dir). Update the notebook logic around the
directory/root_dir creation (the variable directory and root_dir) to check the
MONAI_DATA_DIRECTORY env var and raise a clear error if it's None (with guidance
to set MONAI_DATA_DIRECTORY to the fastMRI data path), and remove the
unconditional shutil.rmtree(root_dir) cleanup for the fallback case; ensure the
error is raised before any downstream operations (the cells that previously
error out) so the user gets a clear prompt rather than the notebook silently
creating and later deleting a useless temp dir.
git push -u origin add-kspace-basics-tutorial
Adds an introductory notebook covering k-space fundamentals, Fourier transform connection, aliasing from undersampling, and MONAI's reconstruction transforms using the fastMRI knee single-coil dataset. Also adds the missing Reconstruction section to the main README.
Fixes # .
Description
A few sentences describing the changes proposed in this pull request.
Checks
./figurefolder./runner.sh -t <path to .ipynb file>Summary by CodeRabbit
New Features
Documentation