Skip to content

Conversation

@ZiyueXu77
Copy link
Collaborator

Fixes # .

Description

A few sentences describing the changes proposed in this pull request.

Types of changes

  • Non-breaking change (fix or new feature that would not break existing functionality).
  • Breaking change (fix or new feature that would cause existing functionality to change).
  • New tests added to cover the changes.
  • Quick tests passed locally by running ./runtest.sh.
  • In-line docstrings updated.
  • Documentation updated.

Copilot AI review requested due to automatic review settings January 23, 2026 15:05
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Jan 23, 2026

Greptile Overview

Greptile Summary

This PR adds a reference implementation of the 2019 MLMI paper "Privacy-preserving Federated Brain Tumour Segmentation" to the research directory. The implementation uses NVFlare's Job Recipe API with MONAI for medical image segmentation on the BraTS18 dataset.

Key additions:

  • Complete federated learning setup with 4-client data splits (60-61 training samples each, shared 43 validation samples)
  • Client training script (client.py) with proper error handling for empty data loaders (lines 188, 249)
  • Job configuration (job.py) supporting both standard FedAvg and privacy-preserving SVT differential privacy
  • Model wrapper (model.py) that stores constructor arguments for NVFlare serialization
  • Comprehensive documentation with installation, usage, and experimental results
  • Evaluation scripts and TensorBoard visualization tools
  • Data split JSON files for reproducible experiments

Code quality:

  • Follows custom instruction rule (783565ac-d530-4d49-a8bc-55877cb0a0cd) by raising ValueError with descriptive messages when data loaders are empty
  • Copyright years correctly use 2023 for existing evaluation scripts and 2026 for new implementation files
  • Proper .gitignore to exclude dataset files while keeping datalist JSONs

Confidence Score: 5/5

  • This PR is safe to merge with no issues found
  • All files are well-structured research code with proper error handling, comprehensive documentation, and correct integration with the existing research directory. The implementation follows best practices including explicit error raising for empty data loaders (per custom rule 783565ac), appropriate copyright years, proper .gitignore configuration, and clear separation of concerns across files.
  • No files require special attention

Important Files Changed

Filename Overview
research/README.md Added BraTS18 reference implementation to research list, renumbered existing entries from reverse chronological to chronological order
research/brats18/README.md Comprehensive documentation for BraTS18 federated learning implementation with privacy-preserving features, includes setup, usage, and experimental results
research/brats18/client.py Client-side training script with MONAI medical imaging pipelines, implements validation with proper error handling for empty data loaders (lines 188, 249)
research/brats18/job.py FedAvg recipe configuration with optional SVT differential privacy, handles centralized vs federated training modes
research/brats18/model.py SegResNet wrapper that stores constructor arguments as attributes for NVFlare serialization compatibility
research/brats18/result_stat/brats_3d_test_only.py Evaluation script for testing trained models, includes proper error handling for empty validation metrics (line 140-141)

Sequence Diagram

sequenceDiagram
    participant User
    participant JobScript as job.py
    participant Server as NVFlare Server
    participant Client1 as Client (site-1)
    participant Client2 as Client (site-2)
    participant ClientN as Client (site-N)
    
    User->>JobScript: python job.py --n_clients 4
    JobScript->>JobScript: Create FedAvgRecipe with BratsSegResNet
    JobScript->>JobScript: Configure SVTPrivacy filter (if --enable_dp)
    JobScript->>JobScript: Setup SimEnv with GPU config
    JobScript->>Server: Execute job with initial model
    
    loop For each FL round (600 rounds)
        Server->>Client1: Send global model weights
        Server->>Client2: Send global model weights
        Server->>ClientN: Send global model weights
        
        Client1->>Client1: Load client-specific datalist (site-1.json)
        Client1->>Client1: Validate global model on local data
        Client1->>Client1: Train for aggregation_epochs
        Client1->>Client1: Compute weight differences
        Client1->>Server: Send weight diff + val_dice metric
        
        Client2->>Client2: Load client-specific datalist (site-2.json)
        Client2->>Client2: Validate global model on local data
        Client2->>Client2: Train for aggregation_epochs
        Client2->>Client2: Compute weight differences
        Client2->>Server: Send weight diff + val_dice metric
        
        ClientN->>ClientN: Load client-specific datalist (site-N.json)
        ClientN->>ClientN: Validate global model on local data
        ClientN->>ClientN: Train for aggregation_epochs
        ClientN->>ClientN: Compute weight differences
        ClientN->>Server: Send weight diff + val_dice metric
        
        alt Differential Privacy enabled
            Server->>Server: Apply SVTPrivacy filter to weight diffs
        end
        
        Server->>Server: Weighted average aggregation (FedAvg)
        Server->>Server: Track best model by val_dice
        Server->>Server: Log metrics to TensorBoard
    end
    
    Server->>JobScript: Return best_FL_global_model.pt
    JobScript->>User: Display job status and result path
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

1 file reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds a complete BraTS18 3D brain tumor segmentation research example built on NVFlare’s Recipe API and MONAI, including job configuration, client training, dataset splits, and evaluation utilities.

Changes:

  • Introduces a BratsSegResNet model wrapper, NVFlare FedAvg job configuration (with optional DP), and a client training script using MONAI for BraTS18.
  • Adds BraTS18 dataset structure and per-site datalist JSON splits for federated vs. centralized training, plus a detailed README documenting setup and usage.
  • Provides evaluation and visualization utilities (testing script, TensorBoard event plotting, and requirements files) for comparing centralized, FedAvg, and FedAvg+DP runs.

Reviewed changes

Copilot reviewed 15 out of 17 changed files in this pull request and generated no comments.

Show a summary per file
File Description
research/brats18/.gitignore Ignores large BraTS18 training data, model checkpoints, and Python cache files to keep the repo lightweight.
research/brats18/README.md Documents the BraTS18 federated learning example, including installation, data layout, job configuration, run commands, and result interpretation.
research/brats18/client.py Implements the NVFlare Client API-based training loop using MONAI (data loading, augmentation, Dice loss, optional FedProx, validation, and FL communication).
research/brats18/job.py Configures and runs a FedAvgRecipe job for BraTS18, wiring in the client script, model factory, TensorBoard tracking, and optional SVT-based DP filter.
research/brats18/model.py Defines BratsSegResNet and create_brats_model(), wrapping MONAI’s SegResNet and storing constructor args for NVFlare serialization.
research/brats18/requirements.txt Lists core Python dependencies for training and evaluation (PyTorch, MONAI, tensorboard, etc.).
research/brats18/plot-requirements.txt Lists plotting dependencies used by the TensorBoard event plotting utility (TensorFlow, seaborn, matplotlib).
research/brats18/result_stat/brats_3d_test_only.py Loads a trained SegResNet checkpoint and computes per-class and overall Dice scores on the BraTS18 validation set using MONAI inferers and transforms.
research/brats18/result_stat/plot_tensorboard_events.py Reads TensorBoard event files from simulation workspaces and generates comparison plots for validation metrics across centralized, FedAvg, and FedAvg+DP experiments.
research/brats18/result_stat/testing_models_3d.sh Shell helper script to run brats_3d_test_only.py against the best global models from centralized, FedAvg, and FedAvg+DP jobs in the default simulation workspace.
research/brats18/dataset_brats18/dataset/README.md Briefly instructs users where to place downloaded BraTS18 training data under the local dataset directory.
research/brats18/dataset_brats18/datalist/site-1.json Defines BraTS18 train/validation splits assigned to federated client “site-1” plus shared validation set metadata.
research/brats18/dataset_brats18/datalist/site-2.json Defines BraTS18 train/validation splits assigned to federated client “site-2” plus shared validation set metadata.
research/brats18/dataset_brats18/datalist/site-3.json Defines BraTS18 train/validation splits assigned to federated client “site-3” plus shared validation set metadata.
research/brats18/dataset_brats18/datalist/site-4.json Defines BraTS18 train/validation splits assigned to federated client “site-4” plus shared validation set metadata.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

ZiyueXu77 and others added 2 commits January 23, 2026 10:54
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

No files reviewed, no comments

Edit Code Review Agent Settings | Greptile

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

No files reviewed, no comments

Edit Code Review Agent Settings | Greptile

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

2 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

@ZiyueXu77 ZiyueXu77 requested a review from holgerroth January 23, 2026 16:47
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

No files reviewed, no comments

Edit Code Review Agent Settings | Greptile

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

No files reviewed, no comments

Edit Code Review Agent Settings | Greptile

Copy link
Collaborator

@holgerroth holgerroth left a comment

Choose a reason for hiding this comment

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

Looks good!

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

No files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@ZiyueXu77
Copy link
Collaborator Author

/build

@ZiyueXu77 ZiyueXu77 enabled auto-merge (squash) January 23, 2026 17:16
@ZiyueXu77
Copy link
Collaborator Author

/build

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

No files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@ZiyueXu77 ZiyueXu77 merged commit 1507bda into NVIDIA:2.7 Jan 23, 2026
20 checks passed
@ZiyueXu77 ZiyueXu77 deleted the brats_27 branch January 23, 2026 17:52
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.

2 participants