Skip to content

Conversation

@ZiyueXu77
Copy link
Collaborator

Fixes # .

Description

Apply same updates for KM example to 2.7

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 7, 2026 22:07
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Jan 7, 2026

Greptile Summary

This PR restructures the Kaplan-Meier HE example to support both simulation and production deployment modes using the Recipe API. The changes modernize the example by:

Key Changes:

  • Replaced old km_job.py with new job.py implementing Recipe API for unified sim/prod execution
  • Moved scripts from src/ directory to root: client.py, client_he.py, server.py, server_he.py
  • Changed HE scheme from BFV to CKKS for consistency across simulation and production modes
  • Added production-ready project.yml with HEBuilder configuration for CKKS provisioning
  • Created start_all.sh convenience script for local production testing
  • Updated prepare_he_context.py to support both BFV and CKKS schemes (defaults to CKKS)
  • Extensively updated README with detailed simulation and production setup instructions

Architecture:

  • Simulation mode: Uses manually prepared HE context (base64-encoded .txt files)
  • Production mode: Uses auto-provisioned HE context (raw binary .tenseal files via startup kits)
  • Both modes use CKKS scheme with matching parameters (poly_modulus=8192, coeff_mod=[60,40,40], scale=2^40)
  • 3-round HE workflow: (1) Collect max indices, (2) Send encrypted histograms, (3) Aggregate and distribute results

Quality:

  • All file migrations are clean with preserved logic and copyright headers
  • Proper handling of HE context formats for both simulation and production
  • Comprehensive documentation with clear step-by-step examples
  • Well-structured shell script with error checking and helpful diagnostics
  • No breaking changes to core functionality

Confidence Score: 5/5

  • This PR is safe to merge with high confidence. Changes are well-structured, properly tested migrations of existing functionality to modern APIs.
  • Score of 5 reflects: (1) Clean file restructuring with preserved logic and copyright headers, (2) Proper dual-mode (simulation/production) HE context handling using well-established NVFlare APIs, (3) Consistent use of CKKS scheme across both modes with matching cryptographic parameters, (4) Comprehensive documentation updates with step-by-step examples, (5) Well-designed Recipe API wrapper enabling flexible job configuration, (6) No syntax or logic errors detected in code review, (7) Proper error handling in shell scripts, (8) No changes to core algorithmic logic - only API modernization and deployment improvements.
  • No files require special attention

Important Files Changed

Filename Overview
examples/advanced/kaplan-meier-he/job.py New job.py replaces km_job.py with Recipe API support for both simulation and production modes. Adds Production environment support with proper HE context path handling. Cleaner API design using Recipe class wrapper.
examples/advanced/kaplan-meier-he/server_he.py Moved from src/kaplan_meier_wf_he.py. Implements HE-encrypted workflow with 3 rounds. Uses CKKS scheme for homomorphic encryption. Properly handles both base64-encoded (simulation) and raw binary (production) HE context formats.
examples/advanced/kaplan-meier-he/client_he.py Moved from src/kaplan_meier_train_he.py. Implements HE-encrypted client workflow with CKKS. Properly handles both simulation and production HE context formats (base64-encoded .txt vs raw binary .tenseal files). Includes proper rounding for CKKS floating-point decryption results.
examples/advanced/kaplan-meier-he/README.md Extensive documentation updates. Changed scheme from BFV to CKKS for consistency. Added comprehensive section on HE Context and Data Management. Added detailed simulation and production mode setup instructions with tables and step-by-step guidance. Documentation is clear and complete.
examples/advanced/kaplan-meier-he/project.yml New project.yml file for production provisioning. Defines 5 clients + 1 server + 1 admin. Configured with HEBuilder for CKKS scheme (poly_modulus_degree=8192, coeff_mod_bit_sizes=[60,40,40], scale=40). No issues detected.

Sequence Diagram

sequenceDiagram
    participant Server as Server (KM_HE)
    participant Client as Client (client_he.py)
    participant HEContext as HE Context (CKKS)

    Note over Server,Client: Round 1: Collect Max Index (Cleartext)
    Server->>Client: Send empty message (round=1)
    Client->>Client: Calculate local max histogram index
    Client->>Server: Send max_idx (cleartext)

    Note over Server,Client: Round 2: Collect Encrypted Histograms
    Server->>Server: Aggregate max indices to get global max
    Server->>Client: Send max_idx_global (cleartext, round=2)
    Client->>HEContext: Encrypt histograms with CKKS
    Client->>Server: Send encrypted histograms (ciphertext)

    Note over Server,Client: Round 3: Distribute Aggregated Results
    Server->>HEContext: Add encrypted vectors (homomorphic addition)
    Server->>Client: Send aggregated encrypted histograms (round=3)
    Client->>HEContext: Decrypt histograms with CKKS
    Client->>Client: Perform Kaplan-Meier analysis
    Client->>Client: Save results (km_curve_fl_he.png, km_global.json)
    Client->>Server: Send completion message
Loading

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 applies updates to the Kaplan-Meier with Homomorphic Encryption example for version 2.7, transitioning from BFV to CKKS encryption scheme and adding production deployment support through the Recipe API.

  • Switches from BFV to CKKS homomorphic encryption scheme for better compatibility with approximate arithmetic
  • Adds production mode support with automatic HE context provisioning via startup kits
  • Refactors job submission from JobAPI to Recipe API pattern for improved flexibility

Reviewed changes

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

Show a summary per file
File Description
utils/prepare_he_context.py Updates default HE scheme to CKKS with poly_modulus_degree=8192 and adds CKKS-specific configuration (coeff_mod_bit_sizes, global_scale)
start_all.sh New convenience script for starting all NVFlare parties (server + 5 clients) in the background for local production testing
server_he.py Adds persistor_id parameter, implements dual-mode HE context file handling (base64/raw binary), switches from BFV to CKKS vector operations, improves logging clarity
server.py Adds persistor_id parameter and refactors histogram aggregation logic for clarity
project.yml New provisioning configuration with HEBuilder for automatic CKKS context generation in production mode
km_job.py Removed in favor of new Recipe API approach
job.py New file implementing Recipe API pattern with KMRecipe class for unified simulation and production mode job execution
client_he.py Updates HE context file path resolution for both modes, switches from BFV to CKKS encryption, adds CKKS-specific float-to-int conversion for histogram decryption, fixes output file paths
client.py Updates output file paths to use job-specific directories, refactors histogram logic to avoid repeated max() calls
README.md Comprehensive documentation updates covering CKKS scheme details, production mode setup, provisioning steps, and deployment instructions
Comments suppressed due to low confidence (1)

examples/advanced/kaplan-meier-he/client_he.py:207

  • Redundant int() conversions are applied here. At lines 197-198, hist_obs_global and hist_cen_global are already converted to lists of integers using list comprehensions with int(round(x)). The subsequent int() calls at lines 204 and 207 when indexing these lists are unnecessary since the elements are already integers.

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

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