Skip to content

Conversation

@ViSaReVe
Copy link

@ViSaReVe ViSaReVe commented Feb 9, 2026

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

  • Avoid including large-size files in the PR.
  • Clean up long text outputs from code cells in the notebook.
  • For security purposes, please check the contents and remove any sensitive info such as user names and private key.
  • Ensure (1) hyperlinks and markdown anchors are working (2) use relative paths for tutorial repo files (3) put figure and graphs in the ./figure folder
  • Notebook runs automatically ./runner.sh -t <path to .ipynb file>

Summary by CodeRabbit

  • New Features

    • Added an MRI Reconstruction tutorial series, including a K‑Space basics notebook using fastMRI knee data.
  • Documentation

    • Main README updated with a new "Reconstruction" section replacing a placeholder.
    • Added a tutorials README describing contents, prerequisites, dataset info, and links to U‑Net and VarNet demos.

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>
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@coderabbitai
Copy link

coderabbitai bot commented Feb 9, 2026

Walkthrough

Adds 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

Cohort / File(s) Summary
Project README
README.md
Replaced placeholder text with a new "Reconstruction" section listing three MRI reconstruction tutorials (K‑Space Basics, U‑Net, VarNet).
Tutorials README
reconstruction/MRI_reconstruction/tutorials/README.md
New folder-level README describing the MRI reconstruction tutorial series, dataset notes (fastMRI knee), prerequisites, and links to related production demos.
Tutorial Notebook
reconstruction/MRI_reconstruction/tutorials/01_kspace_basics_fastmri_knee.ipynb
New comprehensive notebook covering k-space fundamentals, undersampling artifacts, FastMRIReader data loading, MONAI transforms (RandomKspaceMaskd, EquispacedKspaceMaskd), preprocessing pipeline, zero-filled vs. ground-truth reconstructions, visualizations, and cleanup.
Runner Config
runner.sh
Added the new notebook to the list exempted from max_epochs handling for build/run orchestration.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 In k-space fields where frequencies play,
I hop through masks and FFTs today.
FastMRI knees and MONAI's art,
Pixels mend as science and heart. ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description check ⚠️ Warning The description includes the required template sections with a summary of changes. However, the critical housekeeping checklist items are all unchecked, indicating potential incomplete validation before submission. Complete and check off all applicable items in the Checks section, particularly verifying that notebook outputs are clean, file sizes are appropriate, and the notebook runs with runner.sh.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Add k-space basics tutorial for MRI reconstruction' directly and clearly summarizes the main change: adding a new tutorial notebook focused on k-space basics, which is the primary addition in this changeset.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a 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_numpy and to_numpy.

as_numpy (line 462) and to_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�� �����Ǿ� �ֽ��ϴ�.
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

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.

Suggested change
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.

Comment on lines 150 to 159
"# 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)}\")"
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

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.

Suggested change
"# 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.

Copy link

@coderabbitai coderabbitai bot left a 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_numpy is identical to as_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 uses ifftshift(ifft2(fftshift(kspace))) — the shifts are swapped. For even-length dimensions (typical for fastMRI data), fftshift and ifftshift are 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)))

Comment on lines +456 to +478
"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",
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

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.

Comment on lines +753 to +755
"if directory is None:\n",
" shutil.rmtree(root_dir)\n",
"print(\"Cleanup complete!\")"
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

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.

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.

1 participant