-
Notifications
You must be signed in to change notification settings - Fork 228
Add cross-site evaluation utility and examples #3923
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
Greptile SummaryThis PR adds cross-site evaluation (CSE) utility and examples to NVFlare, enabling users to evaluate federated learning models across all client sites without sharing data. The implementation follows a consistent pattern with the existing Key Changes:
Minor Issues:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant Recipe/Job
participant Server
participant Client1
participant Client2
alt Mode 1: Standalone CSE with Pre-trained Models
User->>Recipe/Job: python job.py --mode pretrained
Recipe/Job->>Server: Configure NPModelLocator with pre-trained models
Recipe/Job->>Server: Add ValidationJsonGenerator
Recipe/Job->>Server: Add CrossSiteModelEval controller
Recipe/Job->>Client1: Deploy NPValidator
Recipe/Job->>Client2: Deploy NPValidator
Server->>Server: Load pre-trained models (server_model_1, server_model_2)
Server->>Client1: Distribute all models for validation
Server->>Client2: Distribute all models for validation
Client1->>Client1: Validate each model on local data
Client2->>Client2: Validate each model on local data
Client1->>Server: Return validation metrics
Client2->>Server: Return validation metrics
Server->>Server: Generate cross-site validation matrix
Server->>User: Save results to cross_val_results.json
else Mode 2: Training + CSE
User->>Recipe/Job: python job.py --mode training
Recipe/Job->>Recipe/Job: Create NumpyFedAvgRecipe
Recipe/Job->>Recipe/Job: Add NPValidator to clients
Recipe/Job->>Recipe/Job: Call add_cross_site_evaluation()
Recipe/Job->>Server: Deploy ScatterAndGather controller
Recipe/Job->>Server: Deploy CrossSiteModelEval controller
Recipe/Job->>Client1: Deploy ScriptRunner + NPValidator
Recipe/Job->>Client2: Deploy ScriptRunner + NPValidator
loop Training Rounds
Server->>Client1: Send global model
Server->>Client2: Send global model
Client1->>Client1: Train on local data (client.py)
Client2->>Client2: Train on local data (client.py)
Client1->>Server: Send updated model weights
Client2->>Server: Send updated model weights
Server->>Server: Aggregate weights (FedAvg)
end
Server->>Server: Training complete, start CSE
Server->>Client1: Distribute trained models
Server->>Client2: Distribute trained models
Client1->>Client1: Validate all models
Client2->>Client2: Validate all models
Client1->>Server: Return validation metrics
Client2->>Server: Return validation metrics
Server->>Server: Generate cross-site validation matrix
Server->>User: Save results to cross_val_results.json
end
|
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.
Additional Comments (3)
-
examples/hello-world/hello-numpy-cross-val/job.py, line 90 (link)logic:
ValidationJsonGeneratoris NOT added automatically when using plainFedJob. It's only added automatically byBaseFedJob. You need to explicitly add it here. -
examples/hello-world/hello-pt/README.md, line 32 (link)syntax: Typo: "traiing" should be "training"
-
examples/hello-world/hello-pt/README.md, line 48 (link)syntax: Misplaced
#character after comma - should be a space
9 files reviewed, 3 comments
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.
Pull request overview
This PR adds a cross-site evaluation utility function and examples to NVFlare's Recipe API, enabling users to easily evaluate models across different client sites without sharing data.
Key Changes:
- Added
add_cross_site_evaluation()utility function tonvflare/recipe/utils.pyfor programmatically enabling cross-site model evaluation - Added cross-site evaluation support to the PyTorch hello-world example with a
--cross_site_evalcommand-line flag - Unified the NumPy cross-validation example into a single
job.pywith two modes: standalone CSE and training+CSE
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 24 comments.
Show a summary per file
| File | Description |
|---|---|
| nvflare/recipe/utils.py | Adds add_cross_site_evaluation() utility and MODEL_LOCATOR_REGISTRY for PyTorch and NumPy |
| examples/hello-world/hello-pt/job.py | Adds cross-site evaluation support via --cross_site_eval flag and --train_script parameter |
| examples/hello-world/hello-pt/README.md | Updates documentation with cross-site evaluation instructions and usage examples |
| examples/hello-world/hello-numpy-cross-val/job.py | Replaces multiple job scripts with unified implementation supporting pretrained and training modes |
| examples/hello-world/hello-numpy-cross-val/client.py | Adds NumPy training client script with training and evaluation functions |
| examples/hello-world/hello-numpy-cross-val/generate_pretrain_models.py | Updates comment to clarify cross-site evaluation terminology |
| examples/hello-world/hello-numpy-cross-val/README.md | Comprehensive rewrite documenting both CSE modes and Recipe API usage patterns |
| examples/hello-world/hello-numpy-cross-val/job_train_and_cse.py | Deleted - functionality consolidated into unified job.py |
| examples/hello-world/hello-numpy-cross-val/job_cse.py | Deleted - functionality consolidated into unified job.py |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Additional Comments (1)
-
nvflare/recipe/utils.py, line 107-108 (link)logic:
comp_idsdoesn't exist on plainFedJobobjects - only onBaseFedJob.NumpyFedAvgRecipeuses plainFedJobwhich will causeAttributeErrorat runtime when callingadd_cross_site_evaluationwithpersistor_id=None.
9 files reviewed, 1 comment
|
/build |
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.
Additional Comments (2)
-
nvflare/recipe/utils.py, line 39-50 (link)style:
CLIENT_MODEL_DIRconstant is defined but never used in the PR. Therun_pretrained_cse()function in job.py referencesCLIENT_MODEL_DIRdirectly without using this registry value. -
examples/hello-world/hello-numpy-cross-val/job.py, line 38-39 (link)style:
CLIENT_MODEL_DIRis defined but never used anywhere in the code.
9 files reviewed, 2 comments
| @@ -1,87 +1,246 @@ | |||
| # Hello Numpy Cross-Site Validation | |||
| # Hello NumPy Cross-Site Validation | |||
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.
btw, what's the correct term, cross-site validation or cross-site evaluation?
| add_experiment_tracking(recipe, tracking_type="tensorboard") | ||
|
|
||
| if args.cross_site_eval: | ||
| add_cross_site_evaluation(recipe, model_locator_type="pytorch") |
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 good, much cleaner than before.
| def run_pretrained_cse(n_clients: int): | ||
| """Run standalone cross-site evaluation with pre-trained models. |
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.
suggest "run_cse_only" or "run_cse_with_pretrained"
| # Create a minimal FedJob for CSE-only workflow | ||
| job = FedJob(name="hello-numpy-cse", min_clients=n_clients) | ||
|
|
||
| # Configure model locator with pre-trained model locations | ||
| model_locator_id = job.to_server( | ||
| NPModelLocator( | ||
| model_dir=SERVER_MODEL_DIR, | ||
| model_name={"server_model_1": "server_1.npy", "server_model_2": "server_2.npy"}, | ||
| ) | ||
| ) | ||
|
|
||
| # Add validation JSON generator to save results | ||
| job.to_server(ValidationJsonGenerator()) | ||
|
|
||
| # Add cross-site evaluation controller | ||
| eval_controller = CrossSiteModelEval( | ||
| model_locator_id=model_locator_id, | ||
| submit_model_timeout=600, | ||
| validation_timeout=6000, | ||
| ) | ||
| job.to_server(eval_controller) |
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 this part, i think we still need a CrossSiteEvalRecipe to just do CSE without training.
Then we wont expost FedJob to the users.
| env = ProdEnv() | ||
| recipe.export(env, output_path="/tmp/nvflare/prod/job_config") |
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.
you can execute as well in ProdEnv, need to provide the startup kit path
Add cross-site evaluation utility and examples.
Description
Instead of #3895, this takes into account #3895 for adding cross-site evaluation utility and examples.
Types of changes
./runtest.sh.