diff --git a/docs/user_guide/data_scientist_guide/federated_xgboost/secure_xgboost_user_guide.rst b/docs/user_guide/data_scientist_guide/federated_xgboost/secure_xgboost_user_guide.rst index 2390a85fa7..11cfc33c54 100644 --- a/docs/user_guide/data_scientist_guide/federated_xgboost/secure_xgboost_user_guide.rst +++ b/docs/user_guide/data_scientist_guide/federated_xgboost/secure_xgboost_user_guide.rst @@ -211,6 +211,8 @@ Protection .. code-block:: python + from nvflare.recipe import set_per_site_config + recipe = XGBHorizontalRecipe( name="xgb_higgs_horizontal", min_clients=2, @@ -222,8 +224,8 @@ Protection "eval_metric": "auc", "min_child_weight": 100, # increase for coarser trees and reduced privacy exposure }, - per_site_config=per_site_config, ) + set_per_site_config(recipe, per_site_config) Closest Reconstructed Samples (CreditCard) ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/docs/user_guide/data_scientist_guide/job_recipe.rst b/docs/user_guide/data_scientist_guide/job_recipe.rst index ec5d49f8be..094ed9c980 100644 --- a/docs/user_guide/data_scientist_guide/job_recipe.rst +++ b/docs/user_guide/data_scientist_guide/job_recipe.rst @@ -175,9 +175,9 @@ Per-Site Configuration ---------------------- Some recipes accept site-keyed configuration so that each site can use different -arguments, scripts, data loaders, or validators. For recipes that support this -helper, use ``set_per_site_config`` to attach the site-keyed input after -creating the recipe: +arguments, scripts, or data loaders. Call ``set_per_site_config`` immediately +after constructing the recipe, before adding client configuration, files, +filters, components, or tracking: .. code-block:: python @@ -193,35 +193,34 @@ creating the recipe: env = SimEnv(clients=recipe.configured_sites()) -``configured_sites()`` returns the top-level site names from -``set_per_site_config`` when helper-provided configuration exists. Otherwise, for -backward compatibility, it returns site names from a recipe's constructor -``per_site_config`` when available. It does not infer sites from recipe metadata -such as resource specs, launcher specs, or mandatory clients. It also does not -mean those sites are currently connected, validate production enrollment, or -replace the execution environment. - -``set_per_site_config`` stores the mapping and calls the recipe's per-site -configuration hook. It does not by itself create client targets or rebuild an -already generated job. A recipe must implement the hook for helper-provided -per-site fields to affect generated app configuration, command-line arguments, -data loaders, validators, or other recipe behavior. +The helper validates and stores the mapping; it does not build and then replace +client apps. The recipe materializes its client topology once, before the first +client-targeted customization or before export or execution. Built-in FedAvg +recipes and ``FedEvalRecipe`` create one app per configured site directly. If +per-site configuration is omitted, they create the default ``@ALL`` app at that +same preparation point. XGBoost bagging, horizontal, and vertical recipes add +the required data loader and executor components to each configured site and +must be configured before client customization, export, or execution. The +mapping must be non-empty and define at least ``min_clients`` sites. Reserved +targets such as ``server`` and ``@ALL`` are not site names. + +``configured_sites()`` returns the configured top-level site names. It does not +infer sites from recipe metadata, indicate which clients are connected, validate +production enrollment, or replace the execution environment. .. important:: - The second argument to ``set_per_site_config`` is recipe-specific. The helper - validates only that it is a dictionary whose top-level keys are site names and - whose values are dictionaries. Users must ensure each per-site dictionary uses - fields that the selected recipe understands and can convert into generated app - configuration, command-line arguments, data loaders, validators, or other - recipe-specific settings. - - For example, FedAvg's current per-site support is through the legacy - ``FedAvgRecipe(per_site_config=...)`` constructor argument. That constructor - path understands per-site values such as ``train_args``, ``train_script``, - and ``command``. It does not automatically interpret arbitrary keys such as - ``data_path`` or ``batch_size``. For FedAvg, pass those values through - ``train_args`` unless your recipe explicitly documents another shape. + Each site's dictionary is recipe-specific. FedAvg recipes support + ``train_script``, ``train_args``, ``launch_external_process``, ``command``, + ``framework``, ``server_expected_format``, ``params_transfer_type``, + ``launch_once``, and ``shutdown_timeout``. ``FedEvalRecipe`` supports the + corresponding ``eval_script`` and ``eval_args`` fields plus its launch, + command, and exchange-format overrides. XGBoost recipes require a + ``data_loader`` for every site; bagging also accepts ``lr_scale``. + + The older ``per_site_config=...`` constructor argument remains temporarily + available for compatibility, emits ``FutureWarning``, and delegates to this + helper behavior. New code should use ``set_per_site_config``. No Secrets In Recipe Parameters ------------------------------- diff --git a/docs/user_guide/data_scientist_guide/recipe_api.rst b/docs/user_guide/data_scientist_guide/recipe_api.rst index 3726cb6211..5259d28d77 100644 --- a/docs/user_guide/data_scientist_guide/recipe_api.rst +++ b/docs/user_guide/data_scientist_guide/recipe_api.rst @@ -104,16 +104,13 @@ File packaging helpers: Bundle a file or directory into the generated server app package. Helpers that accept ``clients`` target specific generated client apps. This -requires per-site client apps: construct the recipe with the -``per_site_config`` constructor argument on recipes that support it, and each -name in ``clients`` must match an existing per-site client app. With the -default all-clients topology, targeted calls raise an error rather than -silently dropping the change from the generated job, and unknown site names -raise an error rather than deploying a bare app to that site. Calling -``set_per_site_config`` after construction records the configuration for -``configured_sites()`` but does not yet rebuild an existing all-clients app -into per-site apps; recipes will interpret helper-provided per-site config as -follow-up work. +requires per-site client apps: call ``set_per_site_config`` immediately after +constructing a recipe that supports it. The recipe prepares those apps before +applying the first client-targeted helper, and each name in ``clients`` must +match a configured per-site client app. With the default all-clients topology, +targeted calls raise an error rather than silently dropping the change from the +generated job, and unknown site names raise an error rather than deploying a +bare app to that site. Filter helpers: @@ -170,15 +167,18 @@ Per-Site And Metadata Helpers For NVFlare 2.9, the public Recipe configuration surface also includes: ``set_per_site_config(recipe, config)`` - Provide site-keyed, recipe-specific configuration. Each concrete recipe - interprets the site dictionaries for its own workflow. Nested values become - part of the job definition and must not contain secret values. + Provide non-empty, site-keyed, recipe-specific configuration. Call it once, + immediately after recipe construction and before client customizations. Each + concrete recipe interprets the site dictionaries for its own workflow. + Built-in FedAvg, FedEval, and XGBoost recipes require at least ``min_clients`` + entries. The helper validates and stores the mapping; the recipe creates its + client apps once, immediately before the first client customization or before + export or execution. Nested values become part of the job definition and must + not contain secret values. ``recipe.configured_sites()`` - Return top-level site names from helper-provided per-site config when - present. For backward compatibility, recipes may return site names from - legacy constructor ``per_site_config`` when available. This method does not - indicate that sites are connected or replace the execution environment. + Return top-level site names from applied per-site config. This method does + not indicate that sites are connected or replace the execution environment. ``set_recipe_meta(recipe, key, value)`` Set selected generated job metadata by ``JobMetaKey``. The accepted keys are diff --git a/examples/advanced/experiment-tracking/README.md b/examples/advanced/experiment-tracking/README.md index 5b12f228ab..94ee175a49 100644 --- a/examples/advanced/experiment-tracking/README.md +++ b/examples/advanced/experiment-tracking/README.md @@ -227,9 +227,9 @@ add_experiment_tracking( ) ``` -If sites need different tracking URIs, credentials, or experiment names, construct -the recipe with per-site client apps (for example, -`per_site_config={"site-1": {}, "site-2": {}}`) and call +If sites need different tracking URIs, credentials, or experiment names, call +`set_per_site_config(recipe, {"site-1": {}, "site-2": {}})` immediately +after recipe construction to create per-site client apps, then call `add_experiment_tracking()` once per configuration with `clients=[site_name]`. The `clients` argument requires that per-site topology; it intentionally rejects a recipe whose client app is deployed through the default `@ALL` target. diff --git a/examples/advanced/experiment-tracking/mlflow/hello-pt-mlflow-client/README.md b/examples/advanced/experiment-tracking/mlflow/hello-pt-mlflow-client/README.md index ec511786ee..6cdbd7ec19 100644 --- a/examples/advanced/experiment-tracking/mlflow/hello-pt-mlflow-client/README.md +++ b/examples/advanced/experiment-tracking/mlflow/hello-pt-mlflow-client/README.md @@ -193,15 +193,15 @@ each site. In simulation, all clients share the host filesystem, so use the default `tracking_uri=None` behavior or configure distinct paths if the stores must remain physically separate. -For different configuration at each site, construct the recipe with per-site -client apps and then target each site explicitly: +For different configuration at each site, create per-site client apps before +targeting each site explicitly: ```python +from nvflare.recipe import set_per_site_config + sites = ["site-1", "site-2"] -recipe = FedAvgRecipe( - ..., - per_site_config={site: {} for site in sites}, -) +recipe = FedAvgRecipe(...) +set_per_site_config(recipe, {site: {} for site in sites}) for site in sites: add_experiment_tracking( @@ -217,7 +217,7 @@ for site in sites: ) ``` -Targeted `clients=[...]` placement requires `per_site_config` when the recipe is +Targeted `clients=[...]` placement requires per-site configuration when the recipe is constructed; it cannot split an existing `@ALL` client app after the fact. ### Add Experiment Tags diff --git a/examples/advanced/gnn/README.md b/examples/advanced/gnn/README.md index 75b999c907..010e71971a 100644 --- a/examples/advanced/gnn/README.md +++ b/examples/advanced/gnn/README.md @@ -111,12 +111,15 @@ The recipe is configured in `job.py` with parameters such as: The recipe uses **per-site configuration** (`per_site_config`) to provide site-specific training arguments: ```python +from nvflare.recipe import set_per_site_config + per_site_config = {} for i in range(1, num_clients + 1): site_name = f"site-{i}" per_site_config[site_name] = { "train_args": f"--data_path {data_path} --epochs {epochs_per_round} ..." } +set_per_site_config(recipe, per_site_config) ``` This pattern allows each site to receive customized arguments, making it easy to: diff --git a/examples/advanced/gnn/finance/job.py b/examples/advanced/gnn/finance/job.py index e4c81bc06f..9a952f857f 100644 --- a/examples/advanced/gnn/finance/job.py +++ b/examples/advanced/gnn/finance/job.py @@ -17,7 +17,7 @@ from model import SAGE from nvflare.app_opt.pt.recipes.fedavg import FedAvgRecipe -from nvflare.recipe import ProdEnv, SimEnv +from nvflare.recipe import ProdEnv, SimEnv, set_per_site_config def main(): @@ -124,9 +124,9 @@ def main(): min_clients=args.num_clients, num_rounds=args.num_rounds, train_script="client.py", - per_site_config=per_site_config, key_metric="validation_auc", ) + set_per_site_config(recipe, per_site_config) # Export job print(f"Exporting job to {args.job_dir}") diff --git a/examples/advanced/gnn/protein/job.py b/examples/advanced/gnn/protein/job.py index 23f729a2ec..ed7539a113 100644 --- a/examples/advanced/gnn/protein/job.py +++ b/examples/advanced/gnn/protein/job.py @@ -17,7 +17,7 @@ from torch_geometric.nn import GraphSAGE from nvflare.app_opt.pt.recipes.fedavg import FedAvgRecipe -from nvflare.recipe import ProdEnv, SimEnv +from nvflare.recipe import ProdEnv, SimEnv, set_per_site_config def main(): @@ -122,9 +122,9 @@ def main(): min_clients=args.num_clients, num_rounds=args.num_rounds, train_script="client.py", - per_site_config=per_site_config, key_metric="validation_f1", ) + set_per_site_config(recipe, per_site_config) # Export job print(f"Exporting job to {args.job_dir}") diff --git a/examples/advanced/llm_hf/README.md b/examples/advanced/llm_hf/README.md index 584a979cf9..736faf84f4 100644 --- a/examples/advanced/llm_hf/README.md +++ b/examples/advanced/llm_hf/README.md @@ -99,6 +99,8 @@ Key features: **Recipe-Based Approach:** ```python +from nvflare.recipe import set_per_site_config + # Create recipe with FedAvgRecipe # Model can be class instance or dict config # For pre-trained weights: initial_ckpt="/server/path/to/pretrained.pt" @@ -110,7 +112,6 @@ recipe = FedAvgRecipe( train_script="client.py", server_expected_format=server_expected_format, # "pytorch" or "numpy" launch_external_process=True, - per_site_config=per_site_config, # Site-specific configurations key_metric="neg_eval_loss", ) ``` @@ -133,6 +134,7 @@ per_site_config = { "--nproc_per_node=2 --master_port=8888" } } +set_per_site_config(recipe, per_site_config) ``` **Optional Features:** diff --git a/examples/advanced/llm_hf/job.py b/examples/advanced/llm_hf/job.py index bae7e9fa01..7891f7c378 100644 --- a/examples/advanced/llm_hf/job.py +++ b/examples/advanced/llm_hf/job.py @@ -24,7 +24,7 @@ from nvflare.app_opt.pt.quantization.quantizer import ModelQuantizer from nvflare.app_opt.pt.recipes.fedavg import FedAvgRecipe from nvflare.private.fed.utils.fed_utils import split_gpus -from nvflare.recipe import ProdEnv, SimEnv, add_experiment_tracking +from nvflare.recipe import ProdEnv, SimEnv, add_experiment_tracking, set_per_site_config def define_parser(): @@ -176,9 +176,9 @@ def main(): train_script="client.py", server_expected_format=server_expected_format, launch_external_process=True, # Always use external process for LLM training - per_site_config=per_site_config, key_metric="neg_eval_loss", ) + set_per_site_config(recipe, per_site_config) # Add client params to reduce timeout failures for longer LLM runs recipe.add_client_config({"get_task_timeout": 300, "submit_task_result_timeout": 300}) diff --git a/examples/advanced/medgemma/job.py b/examples/advanced/medgemma/job.py index f141902071..04bdafb2d7 100644 --- a/examples/advanced/medgemma/job.py +++ b/examples/advanced/medgemma/job.py @@ -26,7 +26,7 @@ from data_utils import DEFAULT_MODEL_NAME_OR_PATH from nvflare.app_opt.pt.recipes.fedavg import FedAvgRecipe -from nvflare.recipe import SimEnv, add_experiment_tracking +from nvflare.recipe import SimEnv, add_experiment_tracking, set_per_site_config def define_parser(): @@ -271,7 +271,6 @@ def main(): num_rounds=args.num_rounds, model=model, train_script="client.py", - per_site_config=per_site_config, aggregator=( HLoRAMaxRankAggregator(global_lora_rank=args.global_lora_rank) if args.lora_aggregation == "hlora" @@ -281,6 +280,7 @@ def main(): server_expected_format="pytorch", key_metric="", ) + set_per_site_config(recipe, per_site_config) _configure_timeouts(recipe, client_names) if args.wandb: diff --git a/examples/advanced/multi-gpu/pt/README.md b/examples/advanced/multi-gpu/pt/README.md index 221a4c86d4..5d3e39b709 100644 --- a/examples/advanced/multi-gpu/pt/README.md +++ b/examples/advanced/multi-gpu/pt/README.md @@ -103,14 +103,16 @@ distributed job, while `LOCAL_RANK` is only unique on the current node. ### Multiple Clients on Same Machine When running multiple clients on the same machine, use different master ports: ```python -per_site_config={ +from nvflare.recipe import set_per_site_config + +set_per_site_config(recipe, { "site-1": { "command": "... --master_port=7777", }, "site-2": { "command": "... --master_port=8888", }, -} +}) ``` ## Troubleshooting diff --git a/examples/advanced/multi-gpu/pt/job.py b/examples/advanced/multi-gpu/pt/job.py index 4f4771bbec..6eaf2dacbe 100644 --- a/examples/advanced/multi-gpu/pt/job.py +++ b/examples/advanced/multi-gpu/pt/job.py @@ -21,7 +21,7 @@ from model import Net from nvflare.app_opt.pt.recipes.fedavg import FedAvgRecipe -from nvflare.recipe import SimEnv, add_experiment_tracking +from nvflare.recipe import SimEnv, add_experiment_tracking, set_per_site_config def define_parser(): @@ -52,7 +52,10 @@ def main(): train_script=train_script, train_args=train_args, launch_external_process=True, # DDP modes require external process launch - per_site_config={ + ) + set_per_site_config( + recipe, + { "site-1": { "command": "python3 -m torch.distributed.run --nnodes=1 --nproc_per_node=2 --master_port=7777", }, diff --git a/examples/advanced/qwen3-vl/job.py b/examples/advanced/qwen3-vl/job.py index d2d9aa0285..c0bc977ba0 100644 --- a/examples/advanced/qwen3-vl/job.py +++ b/examples/advanced/qwen3-vl/job.py @@ -22,7 +22,7 @@ import re from nvflare.app_opt.pt.recipes.fedavg import FedAvgRecipe -from nvflare.recipe import SimEnv, add_experiment_tracking +from nvflare.recipe import SimEnv, add_experiment_tracking, set_per_site_config def define_parser(): @@ -187,12 +187,12 @@ def main(): num_rounds=args.num_rounds, model=model, train_script="client.py", - per_site_config=per_site_config, launch_external_process=True, server_expected_format="pytorch", # train_qwen.train() does not emit structured eval metrics for model selection in this example. key_metric="", ) + set_per_site_config(recipe, per_site_config) # Configure timeouts for large model / tensor streaming. _configure_timeouts(recipe, client_names) diff --git a/examples/advanced/recipe-k8s/job.py b/examples/advanced/recipe-k8s/job.py index 5596e26068..cecb4ef9e0 100644 --- a/examples/advanced/recipe-k8s/job.py +++ b/examples/advanced/recipe-k8s/job.py @@ -20,7 +20,7 @@ from nvflare.apis.job_def import JobMetaKey from nvflare.apis.utils.format_check import name_check from nvflare.app_opt.pt.recipes.fedavg import FedAvgRecipe -from nvflare.recipe import ProdEnv, set_recipe_meta +from nvflare.recipe import ProdEnv, set_per_site_config, set_recipe_meta def non_negative_int(value: str) -> int: @@ -185,13 +185,16 @@ def create_recipe(args: argparse.Namespace) -> FedAvgRecipe: min_clients=len(client_sites), num_rounds=args.num_rounds, train_script="client.py", - # Explicit per-site entries target these two clients. Recipe metadata - # describes resources and launchers; it does not select deploy targets. - per_site_config={ + key_metric="accuracy", + ) + # Explicit per-site entries target these two clients. Recipe metadata + # describes resources and launchers; it does not select deploy targets. + set_per_site_config( + recipe, + { args.site_1_name: {"train_args": client_train_args(args, site_index=0, gpus=args.site_1_gpus)}, args.site_2_name: {"train_args": client_train_args(args, site_index=1, gpus=args.site_2_gpus)}, }, - key_metric="accuracy", ) # The client training script and the server-side PyTorch persistor both diff --git a/examples/advanced/sklearn-kmeans/README.md b/examples/advanced/sklearn-kmeans/README.md index ee9e9e5379..43c96ea9ec 100644 --- a/examples/advanced/sklearn-kmeans/README.md +++ b/examples/advanced/sklearn-kmeans/README.md @@ -100,7 +100,9 @@ Modify `calculate_data_splits()` in `job.py` to implement different strategies: Use `per_site_config` to pass `train_args` for per-client configuration: ```python -per_site_config={ +from nvflare.recipe import set_per_site_config + +per_site_config = { "site-1": { "train_args": "--data_path /data/iris.csv --train_start 0 --train_end 40 ..." }, @@ -109,6 +111,7 @@ per_site_config={ }, # ... more sites } +set_per_site_config(recipe, per_site_config) ``` **Alternative: Using Separate Data Files** @@ -121,7 +124,7 @@ Instead of using data ranges, you can split your data into separate files for ea # - /data/site2_iris.csv # - /data/site3_iris.csv -per_site_config={ +per_site_config = { "site-1": { "train_args": "--data_path /data/site1_iris.csv ..." }, @@ -130,6 +133,7 @@ per_site_config={ }, # ... more sites } +set_per_site_config(recipe, per_site_config) ``` ### View Results diff --git a/examples/advanced/sklearn-kmeans/job.py b/examples/advanced/sklearn-kmeans/job.py index 8151423ace..928b3226ab 100644 --- a/examples/advanced/sklearn-kmeans/job.py +++ b/examples/advanced/sklearn-kmeans/job.py @@ -24,7 +24,7 @@ import argparse from nvflare.app_opt.sklearn import KMeansFedAvgRecipe -from nvflare.recipe import SimEnv +from nvflare.recipe import SimEnv, set_per_site_config def define_parser(): @@ -105,9 +105,9 @@ def main(): num_rounds=num_rounds, n_clusters=n_clusters, train_script="client.py", - per_site_config=per_site_config, key_metric="metrics", ) + set_per_site_config(recipe, per_site_config) print("Executing recipe in simulation environment...") env = SimEnv(clients=clients, num_threads=n_clients) diff --git a/examples/advanced/sklearn-linear/README.md b/examples/advanced/sklearn-linear/README.md index 4df08056bb..5b0265303d 100644 --- a/examples/advanced/sklearn-linear/README.md +++ b/examples/advanced/sklearn-linear/README.md @@ -89,13 +89,16 @@ Modify `calculate_data_splits()` in `job.py` to implement different split strate - **Unbalanced splits**: Give clients different amounts of data (e.g., linear, exponential ratios) - **Overlapping validation**: Use different validation sets per client -The key is passing a dict to `train_args`: +Build the site mapping and apply it immediately after recipe construction: ```python -train_args = { - "site-1": "--data_path /data/HIGGS.csv --train_start 1100000 --train_end 3080000 ...", - "site-2": "--data_path /data/HIGGS.csv --train_start 3080000 --train_end 5060000 ...", +from nvflare.recipe import set_per_site_config + +per_site_config = { + "site-1": {"train_args": "--data_path /data/HIGGS.csv --train_start 1100000 --train_end 3080000 ..."}, + "site-2": {"train_args": "--data_path /data/HIGGS.csv --train_start 3080000 --train_end 5060000 ..."}, # ... more sites } +set_per_site_config(recipe, per_site_config) ``` **Alternative: Using Separate Data Files** @@ -108,11 +111,12 @@ Instead of using data ranges, you can split your data into separate files for ea # - /data/site2_HIGGS.csv # - /data/site3_HIGGS.csv -train_args = { - "site-1": "--data_path /data/site1_HIGGS.csv", - "site-2": "--data_path /data/site2_HIGGS.csv", - "site-3": "--data_path /data/site3_HIGGS.csv", +per_site_config = { + "site-1": {"train_args": "--data_path /data/site1_HIGGS.csv"}, + "site-2": {"train_args": "--data_path /data/site2_HIGGS.csv"}, + "site-3": {"train_args": "--data_path /data/site3_HIGGS.csv"}, } +set_per_site_config(recipe, per_site_config) ``` This replaces the old `prepare_job_config.sh` approach with a more flexible Python-based solution. diff --git a/examples/advanced/sklearn-linear/job.py b/examples/advanced/sklearn-linear/job.py index 61bb6368f5..31b701090c 100644 --- a/examples/advanced/sklearn-linear/job.py +++ b/examples/advanced/sklearn-linear/job.py @@ -24,7 +24,7 @@ import argparse from nvflare.app_opt.sklearn import SklearnFedAvgRecipe -from nvflare.recipe import SimEnv +from nvflare.recipe import SimEnv, set_per_site_config def define_parser(): @@ -108,9 +108,9 @@ def main(): "fit_intercept": 1, }, train_script="client.py", - per_site_config=per_site_config, key_metric="AUC", ) + set_per_site_config(recipe, per_site_config) print("Executing recipe in simulation environment...") env = SimEnv(clients=clients, num_threads=n_clients) diff --git a/examples/advanced/sklearn-svm/README.md b/examples/advanced/sklearn-svm/README.md index 797452eeb4..255da80d7b 100644 --- a/examples/advanced/sklearn-svm/README.md +++ b/examples/advanced/sklearn-svm/README.md @@ -99,6 +99,8 @@ Modify `calculate_data_splits()` in `job.py` to implement different strategies: Use `per_site_config` to pass `train_args` for per-client configuration: ```python +from nvflare.recipe import set_per_site_config + per_site_config = { "site-1": { "train_args": "--data_path /data/cancer.csv --train_start 0 --train_end 151 ..." @@ -108,6 +110,7 @@ per_site_config = { }, # ... more sites } +set_per_site_config(recipe, per_site_config) ``` **Alternative: Using Separate Data Files** @@ -132,6 +135,7 @@ per_site_config = { "train_args": "--data_path /data/site3_cancer.csv" } } +set_per_site_config(recipe, per_site_config) # No need to pass --train_start, --train_end, etc. when using separate files ``` @@ -158,6 +162,7 @@ per_site_config = { }, # ... more sites } +set_per_site_config(recipe, per_site_config) ``` **Note:** The default backend is `sklearn`. You only need to specify `--backend cuml` if you want GPU acceleration. diff --git a/examples/advanced/sklearn-svm/job.py b/examples/advanced/sklearn-svm/job.py index d00a611983..168c2e052c 100644 --- a/examples/advanced/sklearn-svm/job.py +++ b/examples/advanced/sklearn-svm/job.py @@ -24,7 +24,7 @@ import argparse from nvflare.app_opt.sklearn import SVMFedAvgRecipe -from nvflare.recipe import SimEnv +from nvflare.recipe import SimEnv, set_per_site_config def define_parser(): @@ -108,9 +108,9 @@ def main(): min_clients=n_clients, kernel=kernel, train_script="client.py", - per_site_config=per_site_config, key_metric="AUC", ) + set_per_site_config(recipe, per_site_config) print("Executing recipe in simulation environment...") print("Note: SVM training only requires 1 round (round 0 for training, round 1 for validation)") diff --git a/examples/advanced/xgboost/README.md b/examples/advanced/xgboost/README.md index 249c3c71ee..68175f2612 100644 --- a/examples/advanced/xgboost/README.md +++ b/examples/advanced/xgboost/README.md @@ -121,6 +121,8 @@ TimberStrike is a model inversion attack that exploits `sum_hessian` values and - **Built-in**: NVFlare removes `sum_hessian` from model transmissions in horizontal tree-based mode, eliminating the attack's primary information source. - **Additional**: Increase `min_child_weight` to raise the minimum sum of instance weight (hessian) required per leaf, resulting in coarser tree structure with fewer splits. The [TimberStrike paper](https://arxiv.org/abs/2506.07605) shows that tree depth (and by extension, number of splits) directly impacts reconstruction accuracy, so reducing tree granularity is expected to limit information exposure. Optimal values are task-dependent; refer to the paper for analysis of the privacy-utility trade-off. This parameter can be added to `xgb_params` in the recipe, for example: ```python + from nvflare.recipe import set_per_site_config + recipe = XGBHorizontalRecipe( name="xgb_higgs_horizontal", min_clients=2, @@ -132,8 +134,8 @@ TimberStrike is a model inversion attack that exploits `sum_hessian` values and "eval_metric": "auc", "min_child_weight": 100, # increase for coarser trees and reduced privacy exposure }, - per_site_config=per_site_config, ) + set_per_site_config(recipe, per_site_config) ``` **Closest Reconstructed Samples (CreditCard)**: diff --git a/examples/advanced/xgboost/fedxgb/README.md b/examples/advanced/xgboost/fedxgb/README.md index 34eae38796..fdf9c11cf5 100644 --- a/examples/advanced/xgboost/fedxgb/README.md +++ b/examples/advanced/xgboost/fedxgb/README.md @@ -163,7 +163,7 @@ python3 job.py --site_num 5 --round_num 100 --split_method uniform **Code example** (`job.py`): ```python from nvflare.app_opt.xgboost.recipes import XGBHorizontalRecipe -from nvflare.recipe import SimEnv +from nvflare.recipe import SimEnv, set_per_site_config from higgs_data_loader import HIGGSDataLoader # Create recipe @@ -185,8 +185,8 @@ recipe = XGBHorizontalRecipe( "objective": "binary:logistic", "eval_metric": "auc", }, - per_site_config=per_site_config, ) +set_per_site_config(recipe, per_site_config) # Run simulation with explicit client list (required when using per_site_config) clients = list(per_site_config.keys()) @@ -237,6 +237,7 @@ Modify `--site_num`, `--split_method`, and `--lr_mode` parameters as needed for **Python API:** ```python from nvflare.app_opt.xgboost.recipes import XGBBaggingRecipe +from nvflare.recipe import set_per_site_config from higgs_data_loader import HIGGSDataLoader # Build per-site configuration with data loaders @@ -255,8 +256,8 @@ recipe = XGBBaggingRecipe( num_rounds=1, num_local_parallel_tree=5, local_subsample=0.8, - per_site_config=per_site_config, ) +set_per_site_config(recipe, per_site_config) # Or Cyclic mode recipe = XGBBaggingRecipe( @@ -264,8 +265,8 @@ recipe = XGBBaggingRecipe( min_clients=5, training_mode="cyclic", num_rounds=100, - per_site_config=per_site_config, ) +set_per_site_config(recipe, per_site_config) # Run with explicit client list (required when using per_site_config) clients = list(per_site_config.keys()) @@ -325,7 +326,7 @@ python3 job_vertical.py --run_psi --run_training **Code example** (`job_vertical.py`): ```python from nvflare.app_opt.xgboost.recipes import XGBVerticalRecipe -from nvflare.recipe import SimEnv +from nvflare.recipe import SimEnv, set_per_site_config from vertical_data_loader import VerticalDataLoader # Create vertical XGBoost recipe @@ -354,6 +355,8 @@ for site_id in range(1, 3): ) per_site_config[f"site-{site_id}"] = {"data_loader": data_loader} +set_per_site_config(recipe, per_site_config) + # Run simulation with explicit client list (required when using per_site_config) clients = list(per_site_config.keys()) env = SimEnv(clients=clients) diff --git a/examples/advanced/xgboost/fedxgb/job.py b/examples/advanced/xgboost/fedxgb/job.py index 203181f094..e2236dda97 100644 --- a/examples/advanced/xgboost/fedxgb/job.py +++ b/examples/advanced/xgboost/fedxgb/job.py @@ -17,7 +17,7 @@ from higgs_data_loader import HIGGSDataLoader from nvflare.app_opt.xgboost.recipes import XGBHorizontalRecipe -from nvflare.recipe import SimEnv +from nvflare.recipe import SimEnv, set_per_site_config def define_parser(): @@ -90,8 +90,8 @@ def main(): early_stopping_rounds=args.early_stopping_rounds, use_gpus=args.use_gpus, xgb_params=xgb_params, - per_site_config=per_site_config, ) + set_per_site_config(recipe, per_site_config) # Run simulation with explicit client list (required when using per_site_config). # num_threads must match number of clients so each client has a dedicated simulator diff --git a/examples/advanced/xgboost/fedxgb/job_tree.py b/examples/advanced/xgboost/fedxgb/job_tree.py index d4671973bc..ab78b7462b 100644 --- a/examples/advanced/xgboost/fedxgb/job_tree.py +++ b/examples/advanced/xgboost/fedxgb/job_tree.py @@ -17,7 +17,7 @@ from higgs_data_loader import HIGGSDataLoader from nvflare.app_opt.xgboost.recipes import XGBBaggingRecipe -from nvflare.recipe import SimEnv +from nvflare.recipe import SimEnv, set_per_site_config def define_parser(): @@ -118,8 +118,8 @@ def main(): use_gpus=args.use_gpus, nthread=args.nthread, lr_mode=args.lr_mode, - per_site_config=per_site_config, ) + set_per_site_config(recipe, per_site_config) # Run simulation with explicit client list (required when using per_site_config) clients = list(per_site_config.keys()) diff --git a/examples/advanced/xgboost/fedxgb/job_vertical.py b/examples/advanced/xgboost/fedxgb/job_vertical.py index 36976198a3..101b98ea08 100644 --- a/examples/advanced/xgboost/fedxgb/job_vertical.py +++ b/examples/advanced/xgboost/fedxgb/job_vertical.py @@ -19,7 +19,7 @@ from nvflare.app_common.psi.recipes.dh_psi import DhPSIRecipe from nvflare.app_opt.xgboost.recipes import XGBVerticalRecipe -from nvflare.recipe import SimEnv +from nvflare.recipe import SimEnv, set_per_site_config def define_parser(): @@ -126,8 +126,8 @@ def run_training_job(args): label_owner=args.label_owner, early_stopping_rounds=args.early_stopping_rounds, xgb_params=xgb_params, - per_site_config=per_site_config, ) + set_per_site_config(recipe, per_site_config) # Run training with explicit client list (required when using per_site_config) clients = list(per_site_config.keys()) diff --git a/examples/advanced/xgboost/fedxgb_secure/README.md b/examples/advanced/xgboost/fedxgb_secure/README.md index 0dc237fb4b..1748a2bbcc 100644 --- a/examples/advanced/xgboost/fedxgb_secure/README.md +++ b/examples/advanced/xgboost/fedxgb_secure/README.md @@ -199,7 +199,7 @@ python job_vertical.py --secure ```python from nvflare.app_opt.xgboost.recipes import XGBVerticalRecipe from nvflare.app_opt.xgboost.histogram_based_v2.csv_data_loader import CSVDataLoader -from nvflare.recipe import SimEnv +from nvflare.recipe import SimEnv, set_per_site_config # Generate client ranks (required for secure training) # Maps each client name to a unique rank (0-indexed) @@ -225,8 +225,8 @@ recipe = XGBVerticalRecipe( "objective": "binary:logistic", "eval_metric": "auc", }, - per_site_config=per_site_config, ) +set_per_site_config(recipe, per_site_config) # Run simulation env = SimEnv(num_clients=3) @@ -254,7 +254,7 @@ python job.py --secure ```python from nvflare.app_opt.xgboost.recipes import XGBHorizontalRecipe from nvflare.app_opt.xgboost.histogram_based_v2.csv_data_loader import CSVDataLoader -from nvflare.recipe import SimEnv +from nvflare.recipe import SimEnv, set_per_site_config # Generate client ranks (required for secure training) # Maps each client name to a unique rank (0-indexed) @@ -279,8 +279,8 @@ recipe = XGBHorizontalRecipe( "objective": "binary:logistic", "eval_metric": "auc", }, - per_site_config=per_site_config, ) +set_per_site_config(recipe, per_site_config) # Export job (simulator run requires additional context setup for secure horizontal) env = SimEnv(num_clients=3) diff --git a/examples/advanced/xgboost/fedxgb_secure/job.py b/examples/advanced/xgboost/fedxgb_secure/job.py index 6f0eb393ee..1d5d195017 100644 --- a/examples/advanced/xgboost/fedxgb_secure/job.py +++ b/examples/advanced/xgboost/fedxgb_secure/job.py @@ -31,7 +31,7 @@ from nvflare.app_opt.xgboost.histogram_based_v2.csv_data_loader import CSVDataLoader from nvflare.app_opt.xgboost.recipes import XGBHorizontalRecipe -from nvflare.recipe import SimEnv +from nvflare.recipe import SimEnv, set_per_site_config def define_parser(): @@ -97,8 +97,8 @@ def main(): secure=args.secure, client_ranks=client_ranks, xgb_params=xgb_params, - per_site_config=per_site_config, ) + set_per_site_config(recipe, per_site_config) # Export and run env = SimEnv(num_clients=args.site_num) diff --git a/examples/advanced/xgboost/fedxgb_secure/job_vertical.py b/examples/advanced/xgboost/fedxgb_secure/job_vertical.py index edfd906358..4841596484 100644 --- a/examples/advanced/xgboost/fedxgb_secure/job_vertical.py +++ b/examples/advanced/xgboost/fedxgb_secure/job_vertical.py @@ -36,7 +36,7 @@ from nvflare.app_opt.xgboost.histogram_based_v2.csv_data_loader import CSVDataLoader from nvflare.app_opt.xgboost.recipes import XGBVerticalRecipe -from nvflare.recipe import SimEnv +from nvflare.recipe import SimEnv, set_per_site_config def define_parser(): @@ -110,8 +110,8 @@ def main(): secure=args.secure, client_ranks=client_ranks, xgb_params=xgb_params, - per_site_config=per_site_config, ) + set_per_site_config(recipe, per_site_config) # Export and run env = SimEnv(num_clients=args.site_num) diff --git a/integration/nemo/examples/peft/job.py b/integration/nemo/examples/peft/job.py index b9339128b1..afd403f4a2 100644 --- a/integration/nemo/examples/peft/job.py +++ b/integration/nemo/examples/peft/job.py @@ -23,7 +23,7 @@ from nvflare.app_opt.pt.file_model_persistor import PTFileModelPersistor from nvflare.app_opt.pt.recipes.fedavg import FedAvgRecipe from nvflare.client.config import ExchangeFormat, TransferType -from nvflare.recipe import SimEnv +from nvflare.recipe import SimEnv, set_per_site_config DEFAULT_MODEL_NAME_OR_PATH = "nvidia/NVIDIA-Nemotron-3-Nano-4B-BF16" DEFAULT_INITIAL_ADAPTER_CKPT = "./models/nemotron3_nano_lora_init.pt" @@ -219,7 +219,6 @@ def create_recipe(args): num_rounds=args.num_rounds, model_persistor=model_persistor, train_script="automodel_peft_client.py", - per_site_config=per_site_config, launch_external_process=True, command="python3 -u", server_expected_format=ExchangeFormat.PYTORCH, @@ -229,6 +228,7 @@ def create_recipe(args): client_memory_gc_rounds=1, cuda_empty_cache=True, ) + set_per_site_config(recipe, per_site_config) recipe.add_client_file("automodel_financial_phrase_dataset.py", clients=client_names) recipe.add_client_file("automodel_adapter_loader.py", clients=client_names) recipe.add_client_config({"max_resends": 3}, clients=client_names) diff --git a/integration/nemo/examples/supervised_fine_tuning/job.py b/integration/nemo/examples/supervised_fine_tuning/job.py index c2fd3694c2..89ae26b3dc 100644 --- a/integration/nemo/examples/supervised_fine_tuning/job.py +++ b/integration/nemo/examples/supervised_fine_tuning/job.py @@ -24,7 +24,7 @@ from nvflare.app_opt.pt.file_model_persistor import PTFileModelPersistor from nvflare.app_opt.pt.recipes.fedavg import FedAvgRecipe from nvflare.client.config import ExchangeFormat, TransferType -from nvflare.recipe import SimEnv +from nvflare.recipe import SimEnv, set_per_site_config DEFAULT_MODEL_NAME_OR_PATH = "nvidia/NVIDIA-Nemotron-3-Nano-4B-BF16" DEFAULT_INITIAL_MODEL_CKPT = "./models/nemotron3_nano_4b_sft_init.pt" @@ -201,7 +201,6 @@ def create_recipe(args): num_rounds=args.num_rounds, model_persistor=model_persistor, train_script=_client_script_resource("automodel_sft_client.py"), - per_site_config=per_site_config, launch_external_process=True, command=client_command, server_expected_format=ExchangeFormat.PYTORCH, @@ -211,6 +210,7 @@ def create_recipe(args): client_memory_gc_rounds=1, cuda_empty_cache=True, ) + set_per_site_config(recipe, per_site_config) recipe.add_client_file(_client_script_resource("automodel_sft_dataset.py"), clients=client_names) recipe.add_client_file(_client_script_resource("automodel_full_model_loader.py"), clients=client_names) recipe.add_client_file(_client_script_resource("model_checkpoint.py"), clients=client_names) diff --git a/nvflare/app_common/np/recipes/fedavg.py b/nvflare/app_common/np/recipes/fedavg.py index 32789fe5b8..214c6985b8 100644 --- a/nvflare/app_common/np/recipes/fedavg.py +++ b/nvflare/app_common/np/recipes/fedavg.py @@ -65,8 +65,8 @@ class NumpyFedAvgRecipe(UnifiedFedAvgRecipe): params_transfer_type (str): How to transfer the parameters. DIFF enables automatic difference calculation for full-model client results. A client's FLModel.params_type remains authoritative. Defaults to TransferType.FULL. - per_site_config: Per-site configuration for the federated learning job. Nested - values become part of the generated job definition and must not contain secrets. + per_site_config: Deprecated constructor form. New code should call + ``set_per_site_config(recipe, config)`` immediately after construction. launch_once: Whether external process is launched once or per task. Defaults to True. shutdown_timeout: Seconds to wait before shutdown. Defaults to 0.0. key_metric: Metric used to determine if the model is globally best. Defaults to "accuracy". diff --git a/nvflare/app_opt/pt/recipes/fedavg.py b/nvflare/app_opt/pt/recipes/fedavg.py index d6fa600adf..d665ad48bb 100644 --- a/nvflare/app_opt/pt/recipes/fedavg.py +++ b/nvflare/app_opt/pt/recipes/fedavg.py @@ -67,8 +67,8 @@ class FedAvgRecipe(UnifiedFedAvgRecipe): Defaults to TransferType.FULL. model_persistor: Custom model persistor. If None, PTFileModelPersistor will be used. model_locator: Custom model locator. If None, PTFileModelLocator will be used. - per_site_config: Per-site configuration for the federated learning job. Nested - values become part of the generated job definition and must not contain secrets. + per_site_config: Deprecated constructor form. New code should call + ``set_per_site_config(recipe, config)`` immediately after construction. launch_once: Whether external process is launched once or per task. Defaults to True. shutdown_timeout: Seconds to wait before shutdown. Defaults to 0.0. key_metric: Metric used to determine if the model is globally best. Defaults to "accuracy". diff --git a/nvflare/app_opt/pt/recipes/fedeval.py b/nvflare/app_opt/pt/recipes/fedeval.py index a5917076b5..5bb6406c7b 100644 --- a/nvflare/app_opt/pt/recipes/fedeval.py +++ b/nvflare/app_opt/pt/recipes/fedeval.py @@ -21,7 +21,7 @@ from nvflare.job_config.base_fed_job import BaseFedJob from nvflare.job_config.script_runner import FrameworkType, ScriptRunner from nvflare.recipe.spec import Recipe -from nvflare.recipe.utils import validate_ckpt +from nvflare.recipe.utils import _apply_legacy_constructor_config, _validate_per_site_targets, validate_ckpt # Internal validator @@ -83,8 +83,9 @@ class FedEvalRecipe(Recipe): server_expected_format: What format to exchange the parameters between server and client. Defaults to ExchangeFormat.NUMPY. validation_timeout: Timeout for evaluation task in seconds. Defaults to 6000. - per_site_config: Per-site configuration for the evaluation job. Dictionary mapping - site names to configuration dicts. Each config dict can contain optional overrides: + per_site_config: Deprecated constructor form of per-site configuration. New code should call + ``set_per_site_config(recipe, config)`` immediately after construction. Each config dict can + contain optional overrides: eval_script, eval_args, launch_external_process, command, server_expected_format. Values are stored in the job definition and must not contain actual secret values. If not provided, the same configuration will be used for all clients. Defaults to None. @@ -151,7 +152,8 @@ def __init__( self.command = command self.server_expected_format = server_expected_format self.validation_timeout = validation_timeout - self.per_site_config = per_site_config + legacy_per_site_config = per_site_config + self.per_site_config = None self.client_memory_gc_rounds = client_memory_gc_rounds self.cuda_empty_cache = cuda_empty_cache @@ -190,37 +192,37 @@ def __init__( controller = EvalController(persistor_id=persistor_id, timeout=self.validation_timeout) job.to_server(controller) - # Add client executors - if self.per_site_config is not None: - for site_name, site_config in self.per_site_config.items(): - script = site_config.get("eval_script", self.eval_script) - script_args = site_config.get("eval_args", self.eval_args) - launch_external = site_config.get("launch_external_process", self.launch_external_process) - cmd = site_config.get("command", self.command) - expected_format = site_config.get("server_expected_format", self.server_expected_format) - - executor = ScriptRunner( - script=script, - script_args=script_args, - launch_external_process=launch_external, - command=cmd, - framework=FrameworkType.PYTORCH, - server_expected_format=expected_format, - memory_gc_rounds=self.client_memory_gc_rounds, - cuda_empty_cache=self.cuda_empty_cache, - ) - job.to(executor, site_name) - else: - executor = ScriptRunner( - script=self.eval_script, - script_args=self.eval_args, - launch_external_process=self.launch_external_process, - command=self.command, - framework=FrameworkType.PYTORCH, - server_expected_format=self.server_expected_format, - memory_gc_rounds=self.client_memory_gc_rounds, - cuda_empty_cache=self.cuda_empty_cache, - ) - job.to_clients(executor) - Recipe.__init__(self, job) + + if legacy_per_site_config is not None: + _apply_legacy_constructor_config(self, legacy_per_site_config) + + def _create_client_runner(self, site_config: Dict) -> ScriptRunner: + return ScriptRunner( + script=site_config.get("eval_script", self.eval_script), + script_args=site_config.get("eval_args", self.eval_args), + launch_external_process=site_config.get("launch_external_process", self.launch_external_process), + command=site_config.get("command", self.command), + framework=FrameworkType.PYTORCH, + server_expected_format=site_config.get("server_expected_format", self.server_expected_format), + memory_gc_rounds=self.client_memory_gc_rounds, + cuda_empty_cache=self.cuda_empty_cache, + ) + + def _apply_per_site_config(self, config: Dict[str, Dict]) -> None: + _validate_per_site_targets(config, self.min_clients) + for site_config in config.values(): + self._create_client_runner(site_config) + self.per_site_config = config + + def _prepare_client_apps(self) -> None: + if self.per_site_config is None: + self._job.to_clients(self._create_client_runner({})) + return + + runners = { + site_name: self._create_client_runner(site_config) + for site_name, site_config in self.per_site_config.items() + } + for site_name, runner in runners.items(): + self._job.to(runner, site_name) diff --git a/nvflare/app_opt/sklearn/recipes/fedavg.py b/nvflare/app_opt/sklearn/recipes/fedavg.py index 61b4c5518e..3f2626ba3f 100644 --- a/nvflare/app_opt/sklearn/recipes/fedavg.py +++ b/nvflare/app_opt/sklearn/recipes/fedavg.py @@ -56,10 +56,9 @@ class SklearnFedAvgRecipe(UnifiedFedAvgRecipe): launch_external_process: Whether to launch the script in external process. Defaults to False. command: If launch_external_process=True, command to run script (prepended to script). Defaults to "python3 -u". - per_site_config: Per-site configuration for the federated learning job. Dictionary mapping - site names to configuration dicts. If not provided, the same configuration will be used - for all clients. Nested values become part of the generated job definition and must not - contain secrets. + per_site_config: Deprecated constructor form of per-site configuration. New code should call + ``set_per_site_config(recipe, config)`` immediately after construction. Nested values become + part of the generated job definition and must not contain secrets. key_metric: Metric used to determine if the model is globally best. If validation metrics are a dict, key_metric selects the metric used for global model selection. Defaults to "accuracy". launch_once: Whether the external process will be launched only once at the beginning @@ -97,6 +96,7 @@ class SklearnFedAvgRecipe(UnifiedFedAvgRecipe): ```python from nvflare.app_opt.sklearn import SklearnFedAvgRecipe + from nvflare.recipe import set_per_site_config recipe = SklearnFedAvgRecipe( name="sklearn_linear", @@ -104,7 +104,10 @@ class SklearnFedAvgRecipe(UnifiedFedAvgRecipe): num_rounds=50, model_params={"n_classes": 2, "learning_rate": "constant", "eta0": 1e-4}, train_script="client.py", - per_site_config={ + ) + set_per_site_config( + recipe, + { "site-1": {"train_args": "--data_path /tmp/data/site1.csv"}, "site-2": {"train_args": "--data_path /tmp/data/site2.csv"}, "site-3": {"train_args": "--data_path /tmp/data/site3.csv"}, diff --git a/nvflare/app_opt/sklearn/recipes/kmeans.py b/nvflare/app_opt/sklearn/recipes/kmeans.py index 7b9da9d388..327a47f76f 100644 --- a/nvflare/app_opt/sklearn/recipes/kmeans.py +++ b/nvflare/app_opt/sklearn/recipes/kmeans.py @@ -79,10 +79,9 @@ class KMeansFedAvgRecipe(FedAvgRecipe): launch_external_process: Whether to launch the script in external process. Defaults to False. command: If launch_external_process=True, command to run script (prepended to script). Defaults to "python3 -u". - per_site_config: Per-site configuration for the federated learning job. Dictionary mapping - site names to configuration dicts. If not provided, the same configuration will be used - for all clients. Nested values become part of the generated job definition and must not - contain secrets. + per_site_config: Deprecated constructor form of per-site configuration. New code should call + ``set_per_site_config(recipe, config)`` immediately after construction. Nested values become + part of the generated job definition and must not contain secrets. key_metric: Metric used to determine if the model is globally best. If validation metrics are a dict, key_metric selects the metric used for global model selection. Defaults to "metrics" (which corresponds to the homogeneity score sent by the K-Means client). @@ -110,6 +109,7 @@ class KMeansFedAvgRecipe(FedAvgRecipe): ```python from nvflare.app_opt.sklearn import KMeansFedAvgRecipe + from nvflare.recipe import set_per_site_config recipe = KMeansFedAvgRecipe( name="kmeans_iris", @@ -117,7 +117,10 @@ class KMeansFedAvgRecipe(FedAvgRecipe): num_rounds=5, n_clusters=3, train_script="src/kmeans_fl.py", - per_site_config={ + ) + set_per_site_config( + recipe, + { "site-1": {"train_args": "--data_path /tmp/data/site1.csv --train_start 0 --train_end 50"}, "site-2": {"train_args": "--data_path /tmp/data/site2.csv --train_start 50 --train_end 100"}, "site-3": {"train_args": "--data_path /tmp/data/site3.csv --train_start 100 --train_end 150"}, diff --git a/nvflare/app_opt/sklearn/recipes/svm.py b/nvflare/app_opt/sklearn/recipes/svm.py index d9ea20495b..1e06415371 100644 --- a/nvflare/app_opt/sklearn/recipes/svm.py +++ b/nvflare/app_opt/sklearn/recipes/svm.py @@ -78,10 +78,9 @@ class SVMFedAvgRecipe(FedAvgRecipe): launch_external_process: Whether to launch the script in external process. Defaults to False. command: If launch_external_process=True, command to run script (prepended to script). Defaults to "python3 -u". - per_site_config: Per-site configuration for the federated learning job. Dictionary mapping - site names to configuration dicts. If not provided, the same configuration will be used - for all clients. Nested values become part of the generated job definition and must not - contain secrets. + per_site_config: Deprecated constructor form of per-site configuration. New code should call + ``set_per_site_config(recipe, config)`` immediately after construction. Nested values become + part of the generated job definition and must not contain secrets. key_metric: Metric used to determine if the model is globally best. If validation metrics are a dict, key_metric selects the metric used for global model selection. Defaults to "AUC" (which corresponds to the ROC AUC score sent by the SVM client in round 1). @@ -108,13 +107,17 @@ class SVMFedAvgRecipe(FedAvgRecipe): ```python from nvflare.app_opt.sklearn import SVMFedAvgRecipe + from nvflare.recipe import set_per_site_config recipe = SVMFedAvgRecipe( name="svm_cancer", min_clients=3, kernel="rbf", train_script="client.py", - per_site_config={ + ) + set_per_site_config( + recipe, + { "site-1": {"train_args": "--data_path /tmp/data/site1.csv --train_start 0 --train_end 100"}, "site-2": {"train_args": "--data_path /tmp/data/site2.csv --train_start 100 --train_end 200"}, "site-3": {"train_args": "--data_path /tmp/data/site3.csv --train_start 200 --train_end 300"}, diff --git a/nvflare/app_opt/tf/recipes/fedavg.py b/nvflare/app_opt/tf/recipes/fedavg.py index 78ef210a64..8d1c79f7ec 100644 --- a/nvflare/app_opt/tf/recipes/fedavg.py +++ b/nvflare/app_opt/tf/recipes/fedavg.py @@ -65,8 +65,9 @@ class FedAvgRecipe(UnifiedFedAvgRecipe): calculation for full-model client results. A client's FLModel.params_type remains authoritative. Defaults to TransferType.FULL. model_persistor: Custom model persistor. If None, TFModelPersistor will be used. - per_site_config: Per-site configuration for the federated learning job. Dictionary mapping - site names to configuration dicts. Each config dict can contain optional overrides: + per_site_config: Deprecated constructor form. New code should call + ``set_per_site_config(recipe, config)`` immediately after construction. Each config dict can + contain optional overrides: train_script, train_args, launch_external_process, command, framework, server_expected_format, params_transfer_type, launch_once, shutdown_timeout. Nested values become part of the generated job definition and must not contain secrets. diff --git a/nvflare/app_opt/xgboost/recipes/bagging.py b/nvflare/app_opt/xgboost/recipes/bagging.py index 0d4c0d6454..4aa1e5c0b3 100644 --- a/nvflare/app_opt/xgboost/recipes/bagging.py +++ b/nvflare/app_opt/xgboost/recipes/bagging.py @@ -24,6 +24,7 @@ from nvflare.app_opt.xgboost.tree_based.shareable_generator import XGBModelShareableGenerator from nvflare.job_config.api import FedJob from nvflare.recipe.spec import Recipe +from nvflare.recipe.utils import _apply_legacy_constructor_config, _validate_per_site_targets # Internal — not part of the public API @@ -100,18 +101,15 @@ class XGBBaggingRecipe(Recipe): lr_mode (str, optional): Learning rate mode ("uniform" or "scaled"). Default is "uniform". save_name (str, optional): Model save name. Default is "xgboost_model.json". data_loader_id (str, optional): ID of the data loader component. Default is "dataloader". - per_site_config (dict, optional): Per-site configuration mapping site names to config dicts. - Each config dict must contain 'data_loader' key with XGBDataLoader instance. - Can optionally include 'lr_scale' for scaled learning rate mode. - Nested values become part of the generated job definition and must not contain secrets. - Example: {"site-1": {"data_loader": CSVDataLoader(...), "lr_scale": 0.5}, "site-2": {...}} + per_site_config (dict, optional): Deprecated constructor form of per-site configuration. + New code should call ``set_per_site_config(recipe, config)`` immediately after construction. Example: .. code-block:: python from nvflare.app_opt.xgboost.recipes import XGBBaggingRecipe from nvflare.app_opt.xgboost.histogram_based_v2.csv_data_loader import CSVDataLoader - from nvflare.recipe import SimEnv + from nvflare.recipe import SimEnv, set_per_site_config # Bagging mode (federated Random Forest) with uniform learning rate recipe = XGBBaggingRecipe( @@ -121,7 +119,10 @@ class XGBBaggingRecipe(Recipe): num_rounds=1, num_local_parallel_tree=5, local_subsample=0.5, - per_site_config={ + ) + set_per_site_config( + recipe, + { "site-1": {"data_loader": CSVDataLoader(folder="/tmp/data")}, "site-2": {"data_loader": CSVDataLoader(folder="/tmp/data")}, "site-3": {"data_loader": CSVDataLoader(folder="/tmp/data")}, @@ -134,7 +135,10 @@ class XGBBaggingRecipe(Recipe): min_clients=3, training_mode="bagging", lr_mode="scaled", - per_site_config={ + ) + set_per_site_config( + recipe, + { "site-1": {"data_loader": CSVDataLoader(folder="/tmp/data"), "lr_scale": 0.5}, "site-2": {"data_loader": CSVDataLoader(folder="/tmp/data"), "lr_scale": 0.3}, "site-3": {"data_loader": CSVDataLoader(folder="/tmp/data"), "lr_scale": 0.2}, @@ -210,22 +214,20 @@ def __init__( self.lr_mode = v.lr_mode self.save_name = v.save_name self.data_loader_id = v.data_loader_id - self.per_site_config = per_site_config + legacy_per_site_config = per_site_config + self.per_site_config = None if self.training_mode == "cyclic" and self.min_clients < 2: raise ValueError("Cyclic training requires at least 2 clients (min_clients >= 2).") - # Validate per_site_config is provided - if per_site_config is None: - raise ValueError( - "per_site_config is required for XGBBaggingRecipe. " - "Each site must specify a 'data_loader' in the config dictionary." - ) - - # Configure the job + # Configure site-independent job components first. Site apps are added + # once per-site configuration is known. job = self.configure() Recipe.__init__(self, job) + if legacy_per_site_config is not None: + _apply_legacy_constructor_config(self, legacy_per_site_config) + def configure(self): """Configure the federated job for XGBoost tree-based training.""" # Create FedJob @@ -270,38 +272,45 @@ def configure(self): aggregator = XGBBaggingAggregator() job.to_server(aggregator, id="aggregator") - # Add executors and data loaders per site + return job + + def _add_client_components(self, job: FedJob, site_name: str, site_config: dict) -> None: from nvflare.app_opt.xgboost.tree_based.executor import FedXGBTreeExecutor - # Site-specific executors and data loaders + executor = FedXGBTreeExecutor( + data_loader_id=self.data_loader_id, + training_mode=self.training_mode, + num_client_bagging=self.num_client_bagging, + num_local_parallel_tree=self.num_local_parallel_tree, + local_subsample=self.local_subsample, + local_model_path="model.json", + global_model_path="model_global.json", + learning_rate=self.learning_rate, + objective=self.objective, + max_depth=self.max_depth, + eval_metric=self.eval_metric, + tree_method=self.tree_method, + use_gpus=self.use_gpus, + nthread=self.nthread, + lr_scale=site_config.get("lr_scale", 1.0), + lr_mode=self.lr_mode, + ) + job.to(executor, site_name, id="xgb_tree_executor") + job.to(site_config["data_loader"], site_name, id=self.data_loader_id) + + def _apply_per_site_config(self, config: dict[str, dict]) -> None: + _validate_per_site_targets(config, self.min_clients) + for site_name, site_config in config.items(): + if site_config.get("data_loader") is None: + raise ValueError(f"per_site_config for {site_name!r} must include 'data_loader' key") + + self.per_site_config = config + + def _prepare_client_apps(self) -> None: + self._validate_before_use() for site_name, site_config in self.per_site_config.items(): - data_loader = site_config.get("data_loader") - if data_loader is None: - raise ValueError(f"per_site_config for '{site_name}' must include 'data_loader' key") - - # Get lr_scale from config, default to 1.0 - lr_scale = site_config.get("lr_scale", 1.0) - - # Create executor for this site - executor = FedXGBTreeExecutor( - data_loader_id=self.data_loader_id, - training_mode=self.training_mode, - num_client_bagging=self.num_client_bagging, - num_local_parallel_tree=self.num_local_parallel_tree, - local_subsample=self.local_subsample, - local_model_path="model.json", - global_model_path="model_global.json", - learning_rate=self.learning_rate, - objective=self.objective, - max_depth=self.max_depth, - eval_metric=self.eval_metric, - tree_method=self.tree_method, - use_gpus=self.use_gpus, - nthread=self.nthread, - lr_scale=lr_scale, - lr_mode=self.lr_mode, - ) - job.to(executor, site_name, id="xgb_tree_executor") - job.to(data_loader, site_name, id=self.data_loader_id) + self._add_client_components(self._job, site_name, site_config) - return job + def _validate_before_use(self) -> None: + if not self.configured_sites(): + raise RuntimeError("XGBBaggingRecipe requires set_per_site_config() before export or execution") diff --git a/nvflare/app_opt/xgboost/recipes/histogram.py b/nvflare/app_opt/xgboost/recipes/histogram.py index b0043f70fb..3e4d895299 100644 --- a/nvflare/app_opt/xgboost/recipes/histogram.py +++ b/nvflare/app_opt/xgboost/recipes/histogram.py @@ -24,6 +24,7 @@ from nvflare.app_opt.xgboost.histogram_based_v2.fed_executor import FedXGBHistogramExecutor from nvflare.job_config.api import FedJob from nvflare.recipe.spec import Recipe +from nvflare.recipe.utils import _apply_legacy_constructor_config, _validate_per_site_targets # Internal — not part of the public API @@ -80,17 +81,15 @@ class XGBHorizontalRecipe(Recipe): tree_method='hist', nthread=16. data_loader_id (str, optional): ID of the data loader component. Default is 'dataloader'. metrics_writer_id (str, optional): ID of the metrics writer component. Default is 'metrics_writer'. - per_site_config (dict): Per-site configuration mapping site names to config dicts. - Each config dict must contain 'data_loader' key with XGBDataLoader instance. - Nested values become part of the generated job definition and must not contain secrets. - Example: {"site-1": {"data_loader": CSVDataLoader(...)}, "site-2": {...}} + per_site_config (dict, optional): Deprecated constructor form of per-site configuration. + New code should call ``set_per_site_config(recipe, config)`` immediately after construction. Example: .. code-block:: python from nvflare.app_opt.xgboost.recipes import XGBHorizontalRecipe from nvflare.app_opt.xgboost.histogram_based_v2.csv_data_loader import CSVDataLoader - from nvflare.recipe import SimEnv + from nvflare.recipe import SimEnv, set_per_site_config # Build per-site configuration with data loaders per_site_config = { @@ -109,8 +108,8 @@ class XGBHorizontalRecipe(Recipe): "objective": "binary:logistic", "eval_metric": "auc", }, - per_site_config=per_site_config, ) + set_per_site_config(recipe, per_site_config) # Run simulation with explicit client list clients = list(per_site_config.keys()) @@ -118,9 +117,9 @@ class XGBHorizontalRecipe(Recipe): run = recipe.execute(env) Note: - - Data loaders must be configured via per_site_config parameter. - - TensorBoard tracking is automatically configured for both server and clients. - - Executor and metrics components are automatically added to all clients. + - Data loaders must be configured with ``set_per_site_config`` before export or execution. + - TensorBoard tracking is automatically configured for the server and configured sites. + - Executor and metrics components are automatically added to each configured site. """ _UNSUPPORTED_SECRET_REF_ATTRS = Recipe._UNSUPPORTED_SECRET_REF_ATTRS | {"per_site_config"} @@ -174,19 +173,17 @@ def __init__( self.xgb_params = v.xgb_params self.data_loader_id = v.data_loader_id self.metrics_writer_id = v.metrics_writer_id - self.per_site_config = per_site_config + legacy_per_site_config = per_site_config + self.per_site_config = None - # Validate per_site_config is provided - if per_site_config is None: - raise ValueError( - "per_site_config is required for XGBHorizontalRecipe. " - "Each site must specify a 'data_loader' in the config dictionary." - ) - - # Configure the job + # Configure site-independent job components first. Site apps are added + # once per-site configuration is known. job = self.configure() Recipe.__init__(self, job) + if legacy_per_site_config is not None: + _apply_legacy_constructor_config(self, legacy_per_site_config) + def configure(self): """Configure the federated job for XGBoost histogram-based training.""" # Create FedJob @@ -214,7 +211,9 @@ def configure(self): tb_receiver = TBAnalyticsReceiver(tb_folder="tb_events") job.to_server(tb_receiver, id="tb_receiver") - # Prepare common client components + return job + + def _add_client_components(self, job: FedJob, site_name: str, site_config: dict) -> None: executor_params = { "data_loader_id": self.data_loader_id, "metrics_writer_id": self.metrics_writer_id, @@ -222,28 +221,28 @@ def configure(self): if self.secure: executor_params["in_process"] = True - # Add all components per site (executor, metrics, event converter, data loader) - for site_name, site_config in self.per_site_config.items(): - data_loader = site_config.get("data_loader") - if data_loader is None: - raise ValueError(f"per_site_config for '{site_name}' must include 'data_loader' key") - - # Add executor - executor = FedXGBHistogramExecutor(**executor_params) - job.to(executor, site_name, id="xgb_executor") - - # Add metrics writer - metrics_writer = TBWriter(event_type="analytix_log_stats") - job.to(metrics_writer, site_name, id=self.metrics_writer_id) - - # Add event converter - event_to_fed = ConvertToFedEvent( - events_to_convert=["analytix_log_stats"], - fed_event_prefix="fed.", - ) - job.to(event_to_fed, site_name, id="event_to_fed") + job.to(FedXGBHistogramExecutor(**executor_params), site_name, id="xgb_executor") + job.to(TBWriter(event_type="analytix_log_stats"), site_name, id=self.metrics_writer_id) + job.to( + ConvertToFedEvent(events_to_convert=["analytix_log_stats"], fed_event_prefix="fed."), + site_name, + id="event_to_fed", + ) + job.to(site_config["data_loader"], site_name, id=self.data_loader_id) - # Add data loader - job.to(data_loader, site_name, id=self.data_loader_id) + def _apply_per_site_config(self, config: dict[str, dict]) -> None: + _validate_per_site_targets(config, self.min_clients) + for site_name, site_config in config.items(): + if site_config.get("data_loader") is None: + raise ValueError(f"per_site_config for {site_name!r} must include 'data_loader' key") - return job + self.per_site_config = config + + def _prepare_client_apps(self) -> None: + self._validate_before_use() + for site_name, site_config in self.per_site_config.items(): + self._add_client_components(self._job, site_name, site_config) + + def _validate_before_use(self) -> None: + if not self.configured_sites(): + raise RuntimeError("XGBHorizontalRecipe requires set_per_site_config() before export or execution") diff --git a/nvflare/app_opt/xgboost/recipes/vertical.py b/nvflare/app_opt/xgboost/recipes/vertical.py index 8b85ee0958..bdcbb635a1 100644 --- a/nvflare/app_opt/xgboost/recipes/vertical.py +++ b/nvflare/app_opt/xgboost/recipes/vertical.py @@ -24,6 +24,7 @@ from nvflare.app_opt.xgboost.histogram_based_v2.fed_executor import FedXGBHistogramExecutor from nvflare.job_config.api import FedJob from nvflare.recipe.spec import Recipe +from nvflare.recipe.utils import _apply_legacy_constructor_config, _validate_per_site_targets # Internal — not part of the public API @@ -97,28 +98,21 @@ class XGBVerticalRecipe(Recipe): metrics_writer_id (str, optional): ID of the metrics writer component. Default is 'metrics_writer'. in_process (bool, optional): Whether to run in-process (required for vertical). Default is True. model_file_name (str, optional): Model file name. Default is 'test.model.json'. - per_site_config (dict, optional): Per-site configuration mapping site names to config dicts. - Each config dict must contain 'data_loader' key with XGBDataLoader instance. - Nested values become part of the generated job definition and must not contain secrets. - Example: {"site-1": {"data_loader": VerticalDataLoader(...)}, "site-2": {...}} + per_site_config (dict, optional): Deprecated constructor form of per-site configuration. + New code should call ``set_per_site_config(recipe, config)`` immediately after construction. Example: .. code-block:: python from nvflare.app_opt.xgboost.recipes import XGBVerticalRecipe from vertical_data_loader import VerticalDataLoader - from nvflare.recipe import SimEnv + from nvflare.recipe import SimEnv, set_per_site_config # Step 1: Run PSI first (separate job) to get intersection files # ... PSI job execution ... - # Step 2: Create vertical XGBoost recipe with per-site data loaders - recipe = XGBVerticalRecipe( - name="xgb_vertical", - min_clients=2, - num_rounds=100, - label_owner="site-1", # Only site-1 has labels - per_site_config={ + # Step 2: Create vertical XGBoost recipe and configure site data loaders + per_site_config = { "site-1": { "data_loader": VerticalDataLoader( data_split_path="/tmp/data/site-1/higgs.data.csv", @@ -137,7 +131,14 @@ class XGBVerticalRecipe(Recipe): train_proportion=0.8, ) }, + } + recipe = XGBVerticalRecipe( + name="xgb_vertical", + min_clients=2, + num_rounds=100, + label_owner="site-1", # Only site-1 has labels ) + set_per_site_config(recipe, per_site_config) # Step 3: Run with explicit client list clients = list(per_site_config.keys()) @@ -149,8 +150,8 @@ class XGBVerticalRecipe(Recipe): - Only one client should be designated as label_owner - All clients must have overlapping sample IDs (after PSI) - Uses histogram_v2 algorithm with data_split_mode=1 (vertical) - - Data loaders must be configured via per_site_config parameter - - Executor and metrics components are automatically added to all clients + - Data loaders must be configured with ``set_per_site_config`` before export or execution + - Executor and metrics components are automatically added to each configured site - TensorBoard tracking is automatically configured """ @@ -208,56 +209,24 @@ def __init__( self.early_stopping_rounds = v.early_stopping_rounds self.use_gpus = v.use_gpus self.secure = v.secure - self.client_ranks = v.client_ranks + self._requested_client_ranks = dict(v.client_ranks) + self.client_ranks = dict(v.client_ranks) self.xgb_params = v.xgb_params self.data_loader_id = v.data_loader_id self.metrics_writer_id = v.metrics_writer_id self.in_process = v.in_process self.model_file_name = v.model_file_name - self.per_site_config = per_site_config + legacy_per_site_config = per_site_config + self.per_site_config = None - # Validate per_site_config is provided - if per_site_config is None: - raise ValueError( - "per_site_config is required for XGBVerticalRecipe. " - "Each site must specify a 'data_loader' in the config dictionary." - ) - - if self.label_owner not in per_site_config: - raise ValueError(f"label_owner {self.label_owner!r} must be included in per_site_config") - - if self.client_ranks: - expected_clients = set(per_site_config) - ranked_clients = set(self.client_ranks) - if ranked_clients != expected_clients: - raise ValueError( - f"client_ranks must include exactly the per_site_config clients: expected " - f"{sorted(expected_clients)}, got {sorted(ranked_clients)}" - ) - non_integer_ranks = { - client_name: rank - for client_name, rank in self.client_ranks.items() - if not isinstance(rank, int) or isinstance(rank, bool) - } - if non_integer_ranks: - raise ValueError(f"client_ranks values must be integers, got {non_integer_ranks}") - expected_ranks = set(range(len(per_site_config))) - actual_ranks = set(self.client_ranks.values()) - if actual_ranks != expected_ranks: - raise ValueError( - f"client_ranks values must be unique and consecutive from 0 to {len(per_site_config) - 1}: " - f"got {self.client_ranks}" - ) - if self.client_ranks.get(self.label_owner) != 0: - raise ValueError("label_owner must be assigned rank 0 for vertical XGBoost") - else: - ranked_clients = [self.label_owner] + sorted(c for c in per_site_config if c != self.label_owner) - self.client_ranks = {client_name: rank for rank, client_name in enumerate(ranked_clients)} - - # Configure the job + # Configure site-independent job components first. The controller and + # client apps depend on the site mapping and are prepared together later. job = self.configure() Recipe.__init__(self, job) + if legacy_per_site_config is not None: + _apply_legacy_constructor_config(self, legacy_per_site_config) + def configure(self): """Configure the federated job for vertical XGBoost training.""" @@ -265,57 +234,93 @@ def configure(self): job = FedJob(name=self.name, min_clients=self.min_clients) job.to_server(MetricsArtifactWriter(), id="metrics_artifact_writer") - # Configure controller for vertical mode + # Add TensorBoard receiver to server + tb_receiver = TBAnalyticsReceiver(tb_folder="tb_events") + job.to_server(tb_receiver, id="tb_receiver") + + return job + + def _create_controller(self, client_ranks: dict) -> XGBFedController: controller_kwargs = { "num_rounds": self.num_rounds, "data_split_mode": 1, # 1 = vertical (column split) "secure_training": self.secure, "xgb_options": {"early_stopping_rounds": self.early_stopping_rounds, "use_gpus": self.use_gpus}, "xgb_params": self.xgb_params, - "client_ranks": self.client_ranks, + "client_ranks": client_ranks, } # In-process execution is required for secure training. if self.secure: controller_kwargs["in_process"] = True # Required for secure training - controller = XGBFedController(**controller_kwargs) - job.to_server(controller, id="xgb_controller") + return XGBFedController(**controller_kwargs) - # Add TensorBoard receiver to server - - tb_receiver = TBAnalyticsReceiver(tb_folder="tb_events") - job.to_server(tb_receiver, id="tb_receiver") - - # Add executor and components + def _resolve_client_ranks(self, config: dict[str, dict]) -> dict: + if self.label_owner not in config: + raise ValueError(f"label_owner {self.label_owner!r} must be included in per_site_config") - # Add all components per site (executor, metrics, event converter, data loader) - for site_name, site_config in self.per_site_config.items(): - data_loader = site_config.get("data_loader") - if data_loader is None: - raise ValueError(f"per_site_config for {site_name!r} must include 'data_loader' key") + if not self._requested_client_ranks: + ranked_clients = [self.label_owner] + sorted(c for c in config if c != self.label_owner) + return {client_name: rank for rank, client_name in enumerate(ranked_clients)} - # Add executor - executor = FedXGBHistogramExecutor( - data_loader_id=self.data_loader_id, - metrics_writer_id=self.metrics_writer_id, - in_process=True if self.secure else self.in_process, - model_file_name=self.model_file_name, + expected_clients = set(config) + ranked_clients = set(self._requested_client_ranks) + if ranked_clients != expected_clients: + raise ValueError( + f"client_ranks must include exactly the per_site_config clients: expected " + f"{sorted(expected_clients)}, got {sorted(ranked_clients)}" ) - job.to(executor, site_name, id="xgb_hist_executor", tasks=["config", "start"]) + non_integer_ranks = { + client_name: rank + for client_name, rank in self._requested_client_ranks.items() + if not isinstance(rank, int) or isinstance(rank, bool) + } + if non_integer_ranks: + raise ValueError(f"client_ranks values must be integers, got {non_integer_ranks}") + expected_ranks = set(range(len(config))) + actual_ranks = set(self._requested_client_ranks.values()) + if actual_ranks != expected_ranks: + raise ValueError( + f"client_ranks values must be unique and consecutive from 0 to {len(config) - 1}: " + f"got {self._requested_client_ranks}" + ) + if self._requested_client_ranks.get(self.label_owner) != 0: + raise ValueError("label_owner must be assigned rank 0 for vertical XGBoost") + return dict(self._requested_client_ranks) + + def _add_client_components(self, job: FedJob, site_name: str, site_config: dict) -> None: + executor = FedXGBHistogramExecutor( + data_loader_id=self.data_loader_id, + metrics_writer_id=self.metrics_writer_id, + in_process=True if self.secure else self.in_process, + model_file_name=self.model_file_name, + ) + job.to(executor, site_name, id="xgb_hist_executor", tasks=["config", "start"]) + job.to(TBWriter(event_type="analytix_log_stats"), site_name, id=self.metrics_writer_id) + job.to( + ConvertToFedEvent(events_to_convert=["analytix_log_stats"], fed_event_prefix="fed."), + site_name, + id="event_to_fed", + ) + job.to(site_config["data_loader"], site_name, id=self.data_loader_id) - # Add metrics writer - metrics_writer = TBWriter(event_type="analytix_log_stats") - job.to(metrics_writer, site_name, id=self.metrics_writer_id) + def _apply_per_site_config(self, config: dict[str, dict]) -> None: + _validate_per_site_targets(config, self.min_clients) + for site_name, site_config in config.items(): + if site_config.get("data_loader") is None: + raise ValueError(f"per_site_config for {site_name!r} must include 'data_loader' key") - # Add event converter - event_to_fed = ConvertToFedEvent( - events_to_convert=["analytix_log_stats"], - fed_event_prefix="fed.", - ) - job.to(event_to_fed, site_name, id="event_to_fed") + client_ranks = self._resolve_client_ranks(config) + self.client_ranks = client_ranks + self.per_site_config = config - # Add data loader - job.to(data_loader, site_name, id=self.data_loader_id) + def _prepare_client_apps(self) -> None: + self._validate_before_use() + self._job.to_server(self._create_controller(self.client_ranks), id="xgb_controller") + for site_name, site_config in self.per_site_config.items(): + self._add_client_components(self._job, site_name, site_config) - return job + def _validate_before_use(self) -> None: + if not self.configured_sites(): + raise RuntimeError("XGBVerticalRecipe requires set_per_site_config() before export or execution") diff --git a/nvflare/job_config/api.py b/nvflare/job_config/api.py index 6cb5a29127..85084a13f4 100644 --- a/nvflare/job_config/api.py +++ b/nvflare/job_config/api.py @@ -33,6 +33,15 @@ _ADD_TO_JOB_METHOD_NAME = "add_to_fed_job" +def validate_target_name(target: str) -> None: + """Validate a target name accepted by :class:`FedJob`.""" + if not target: + raise ValueError("Must provide a valid target name") + + if any(c in SPECIAL_CHARACTERS for c in target) and target != ALL_SITES: + raise ValueError(f"target {target} name contains invalid character") + + class FedApp: def __init__(self, app_config: Union[ClientAppConfig, ServerAppConfig]): """FedApp handles `ClientAppConfig` and `ServerAppConfig` and allows setting task result or task data filters.""" @@ -568,11 +577,7 @@ def add_file_to_clients(self, src_path: str, dest_dir=None, app_folder_type=None self.add_file_to(src_path, ALL_SITES, dest_dir, app_folder_type) def _validate_target(self, target): - if not target: - raise ValueError("Must provide a valid target name") - - if any(c in SPECIAL_CHARACTERS for c in target) and target != ALL_SITES: - raise ValueError(f"target {target} name contains invalid character") + validate_target_name(target) def _apply_fail_fast(self, server_config: ServerAppConfig): """Inject fail_fast configuration into the server app config. diff --git a/nvflare/recipe/fedavg.py b/nvflare/recipe/fedavg.py index 65d8763e18..90c47065d5 100644 --- a/nvflare/recipe/fedavg.py +++ b/nvflare/recipe/fedavg.py @@ -18,7 +18,6 @@ from pydantic import BaseModel from nvflare.apis.dxo import DataKind -from nvflare.apis.job_def import ALL_SITES, SERVER_SITE_NAME from nvflare.app_common.abstract.aggregator import Aggregator from nvflare.app_common.abstract.model_persistor import ModelPersistor from nvflare.app_common.app_constant import DefaultCheckpointFileName @@ -28,6 +27,7 @@ from nvflare.job_config.base_fed_job import BaseFedJob from nvflare.job_config.script_runner import ScriptRunner from nvflare.recipe.spec import Recipe +from nvflare.recipe.utils import _apply_legacy_constructor_config, _validate_per_site_targets # Internal — not part of the public API @@ -123,8 +123,9 @@ class FedAvgRecipe(Recipe): A client's FLModel.params_type remains authoritative. Defaults to TransferType.FULL. model_persistor: Custom model persistor for any framework. If None, uses the framework's default persistor when one is available. - per_site_config: Per-site configuration for the federated learning job. Dictionary mapping - site names to configuration dicts. Each config dict can contain optional overrides: + per_site_config: Deprecated constructor form of per-site configuration. New code should call + ``set_per_site_config(recipe, config)`` immediately after construction. Each config dict can + contain optional overrides: - train_script (str): Training script path - train_args (str): Script arguments - launch_external_process (bool): Whether to launch external process @@ -276,11 +277,15 @@ def __init__( self.launch_external_process = v.launch_external_process self.command = v.command self.framework = v.framework + # Some wrappers expose a different framework to Recipe utilities after + # construction (for example NumPy exposes RAW for CSE). ScriptRunner + # creation must retain the framework validated for client exchange. + self._client_runner_framework = v.framework self.server_expected_format = v.server_expected_format self.params_transfer_type = v.params_transfer_type self.model_persistor = v.model_persistor - self.per_site_config = v.per_site_config - self._validate_per_site_config(self.per_site_config) + legacy_per_site_config = v.per_site_config + self.per_site_config = None self._validate_aggregator_data_kind() self.launch_once = v.launch_once self.shutdown_timeout = v.shutdown_timeout @@ -358,74 +363,52 @@ def __init__( ) job.to_server(controller) - if self.per_site_config is not None: - for site_name, site_config in self.per_site_config.items(): - # Use site-specific config or fall back to defaults - script = ( - site_config.get("train_script") - if site_config.get("train_script") is not None - else self.train_script - ) - script_args = ( - site_config.get("train_args") if site_config.get("train_args") is not None else self.train_args - ) - launch_external = ( - site_config.get("launch_external_process") - if site_config.get("launch_external_process") is not None - else self.launch_external_process - ) - command = site_config.get("command") if site_config.get("command") is not None else self.command - framework = site_config.get("framework") if site_config.get("framework") is not None else self.framework - expected_format = ( - site_config.get("server_expected_format") - if site_config.get("server_expected_format") is not None - else self.server_expected_format - ) - transfer_type = ( - site_config.get("params_transfer_type") - if site_config.get("params_transfer_type") is not None - else self.params_transfer_type - ) - launch_once = ( - site_config.get("launch_once") if site_config.get("launch_once") is not None else self.launch_once - ) - shutdown_timeout = ( - site_config.get("shutdown_timeout") - if site_config.get("shutdown_timeout") is not None - else self.shutdown_timeout - ) + Recipe.__init__(self, job) - executor = ScriptRunner( - script=script, - script_args=script_args, - launch_external_process=launch_external, - command=command, - framework=framework, - server_expected_format=expected_format, - params_transfer_type=transfer_type, - launch_once=launch_once, - shutdown_timeout=shutdown_timeout, - memory_gc_rounds=self.client_memory_gc_rounds, - cuda_empty_cache=self.cuda_empty_cache, - ) - job.to(executor, site_name) - else: - executor = ScriptRunner( - script=self.train_script, - script_args=self.train_args, - launch_external_process=self.launch_external_process, - command=self.command, - framework=self.framework, - server_expected_format=self.server_expected_format, - params_transfer_type=self.params_transfer_type, - launch_once=self.launch_once, - shutdown_timeout=self.shutdown_timeout, - memory_gc_rounds=self.client_memory_gc_rounds, - cuda_empty_cache=self.cuda_empty_cache, - ) - job.to_clients(executor) + if legacy_per_site_config is not None: + _apply_legacy_constructor_config(self, legacy_per_site_config) - Recipe.__init__(self, job) + @staticmethod + def _site_value(site_config: Dict, key: str, default: Any) -> Any: + value = site_config.get(key) + return default if value is None else value + + def _create_client_runner(self, site_config: Dict) -> ScriptRunner: + return ScriptRunner( + script=self._site_value(site_config, "train_script", self.train_script), + script_args=self._site_value(site_config, "train_args", self.train_args), + launch_external_process=self._site_value( + site_config, "launch_external_process", self.launch_external_process + ), + command=self._site_value(site_config, "command", self.command), + framework=self._site_value(site_config, "framework", self._client_runner_framework), + server_expected_format=self._site_value(site_config, "server_expected_format", self.server_expected_format), + params_transfer_type=self._site_value(site_config, "params_transfer_type", self.params_transfer_type), + launch_once=self._site_value(site_config, "launch_once", self.launch_once), + shutdown_timeout=self._site_value(site_config, "shutdown_timeout", self.shutdown_timeout), + memory_gc_rounds=self.client_memory_gc_rounds, + cuda_empty_cache=self.cuda_empty_cache, + ) + + def _apply_per_site_config(self, config: Dict[str, Dict]) -> None: + self._validate_per_site_config(config) + # Validate every runner override while set_per_site_config() is still + # recoverable; actual client apps are materialized later. + for site_config in config.values(): + self._create_client_runner(site_config) + self.per_site_config = config + + def _prepare_client_apps(self) -> None: + if self.per_site_config is None: + self._job.to_clients(self._create_client_runner({})) + return + + runners = { + site_name: self._create_client_runner(site_config) + for site_name, site_config in self.per_site_config.items() + } + for site_name, runner in runners.items(): + self._job.to(runner, site_name) @staticmethod def _resolve_model_filenames(best_model_filename: Optional[str], save_filename: Optional[str]) -> tuple[str, str]: @@ -445,22 +428,8 @@ def _resolve_model_filenames(best_model_filename: Optional[str], save_filename: ) return save_filename, save_filename - @staticmethod - def _validate_per_site_config(per_site_config: Optional[Dict[str, Dict]]) -> None: - if per_site_config is None: - return - - reserved_targets = {SERVER_SITE_NAME, ALL_SITES} - for site_name, site_config in per_site_config.items(): - if not isinstance(site_name, str): - raise ValueError(f"per_site_config key must be str, got {type(site_name).__name__}") - if site_name in reserved_targets: - raise ValueError( - f"'{site_name}' is a reserved target name and cannot be used in per_site_config. " - f"Reserved names: {sorted(reserved_targets)}" - ) - if not isinstance(site_config, dict): - raise ValueError(f"per_site_config['{site_name}'] must be a dict, got {type(site_config).__name__}") + def _validate_per_site_config(self, per_site_config: Dict[str, Dict]) -> None: + _validate_per_site_targets(per_site_config, self.min_clients) def _validate_aggregator_data_kind(self) -> None: from nvflare.recipe.utils import validate_aggregator_data_kind diff --git a/nvflare/recipe/spec.py b/nvflare/recipe/spec.py index e0f00b0dcf..bbd31ea267 100644 --- a/nvflare/recipe/spec.py +++ b/nvflare/recipe/spec.py @@ -253,7 +253,8 @@ def __init__(self, job: FedJob): """ self._job = job self.name = job.name - self._helper_per_site_config = None + self._configured_site_names = None + self._client_apps_prepared = False self._tensor_streaming_added = False self._cse_added = False warn_on_potential_secrets(getattr(job, "name", None), context="recipe parameter 'name'") @@ -295,11 +296,32 @@ def set_per_site_config(self, config: Dict[str, Dict]) -> None: The generic helper validates only the site-keyed shape. Recipes that need to map fields into generated app config, command arguments, data loaders, or validators should override ``_apply_per_site_config``. + Client topology is prepared later, before the first client-targeted + customization or before export or execution. Per-site config values end up in the generated job configuration in clear text and must never contain actual secret values; see the Recipe class docstring for the recommended alternatives. """ + if not isinstance(config, dict): + raise TypeError(f"per-site config must be a dict, got {type(config).__name__}") + if not config: + raise ValueError("per-site config must not be empty") + for site_name, site_config in config.items(): + if not isinstance(site_name, str): + raise TypeError(f"per-site config key must be a str, got {type(site_name).__name__}") + if not isinstance(site_config, dict): + raise TypeError( + f"per-site config for site {site_name!r} must be a dict, got {type(site_config).__name__}" + ) + if self._configured_site_names is not None: + raise RuntimeError("per-site config has already been applied to this recipe") + if self._client_apps_prepared: + raise RuntimeError( + "per-site config must be applied immediately after recipe construction and before client " + "configuration, files, filters, or components are added" + ) + warn_on_potential_secrets(config, context="per_site_config") if self._SUPPORTED_PER_SITE_SECRET_REF_KEYS is not None: warn_on_unsupported_secret_refs_outside_keys( @@ -310,11 +332,36 @@ def set_per_site_config(self, config: Dict[str, Dict]) -> None: ) else: warn_on_unsupported_secret_refs(config, context="per_site_config") - self._helper_per_site_config = dict(config) - self._apply_per_site_config(dict(self._helper_per_site_config)) + # Copy each site's dictionary so deferred client preparation cannot be + # changed by later mutation of the caller's config. Values such as data + # loader objects intentionally retain their identity. + config_snapshot = {site_name: dict(site_config) for site_name, site_config in config.items()} + configured_site_names = tuple(config_snapshot) + self._apply_per_site_config(config_snapshot) + self._configured_site_names = configured_site_names def _apply_per_site_config(self, config: Dict[str, Dict]) -> None: - """Recipe-specific hook for helper-provided per-site configuration.""" + """Validate and store recipe-specific per-site configuration. + + Client apps must not be added here. Recipes with configurable client + topology should create them in ``_prepare_client_apps`` instead. + """ + pass + + def _prepare_client_apps(self) -> None: + """Create this recipe's client apps before client customization or use.""" + pass + + def _ensure_client_apps_prepared(self) -> None: + """Prepare client apps once, after per-site configuration is known.""" + if self._client_apps_prepared: + return + + self._prepare_client_apps() + self._client_apps_prepared = True + + def _validate_before_use(self) -> None: + """Validate recipe state immediately before export or execution.""" pass def configured_sites(self) -> List[str]: @@ -324,9 +371,9 @@ def configured_sites(self) -> List[str]: metadata, validate production enrollment, or indicate which clients are connected in the execution environment. """ - helper_per_site_config = getattr(self, "_helper_per_site_config", None) - if helper_per_site_config is not None: - return list(helper_per_site_config.keys()) + configured_site_names = getattr(self, "_configured_site_names", None) + if configured_site_names is not None: + return list(configured_site_names) legacy_per_site_config = getattr(self, "per_site_config", None) if isinstance(legacy_per_site_config, dict): @@ -436,6 +483,19 @@ def _add_to_client_apps(self, obj, clients: Optional[List[str]] = None, **kwargs from nvflare.apis.job_def import ALL_SITES, SERVER_SITE_NAME from nvflare.job_config.defs import JobTargetType + # Validate the selector before materializing topology so an invalid call + # does not close the set_per_site_config() configuration window. + if clients is not None: + if not isinstance(clients, list): + raise TypeError(f"clients must be a list of client names, got {type(clients).__name__}") + if not clients: + raise ValueError("clients must not be empty; omit it to apply to all clients") + for client in clients: + if not isinstance(client, str) or client in (ALL_SITES, SERVER_SITE_NAME): + raise ValueError(f"invalid client name {client!r}: client names must name specific client sites") + + self._ensure_client_apps_prepared() + # FedJob has no public API to list per-site deploy targets, so we inspect # private deploy map to preserve existing per-site client topology. deploy_map = getattr(self._job, "_deploy_map", {}) @@ -452,14 +512,6 @@ def _add_to_client_apps(self, obj, clients: Optional[List[str]] = None, **kwargs else: self._job.to_clients(obj, **kwargs) else: - # A bare string would iterate per character; an empty list would be a silent no-op. - if not isinstance(clients, list): - raise TypeError(f"clients must be a list of client names, got {type(clients).__name__}") - if not clients: - raise ValueError("clients must not be empty; omit it to apply to all clients") - for client in clients: - if not isinstance(client, str) or client in (ALL_SITES, SERVER_SITE_NAME): - raise ValueError(f"invalid client name {client!r}: client names must name specific client sites") if ALL_SITES in deploy_map: # The generated job has one client app deployed to all clients. Exporting # both an all-clients app and per-site apps is not expressible (the @@ -467,8 +519,8 @@ def _add_to_client_apps(self, obj, clients: Optional[List[str]] = None, **kwargs # instead of silently losing the placement. raise ValueError( "cannot target specific clients: this recipe's client app applies to all clients. " - "Construct the recipe with per-site client apps (e.g. the per_site_config constructor " - "argument on recipes that support it) or omit clients to apply to all clients." + "Call set_per_site_config immediately after constructing a recipe that supports it, " + "or omit clients to apply to all clients." ) if existing_client_sites: # Targeting a site with no app would create a bare, executor-less app for @@ -794,6 +846,8 @@ class docstring for how to pass references instead. Returns: None """ + self._validate_before_use() + self._ensure_client_apps_prepared() self._warn_potential_secrets_in_params() with self._temporary_exec_params(server_exec_params=server_exec_params, client_exec_params=client_exec_params): if env is not None: @@ -814,6 +868,8 @@ def run( Returns: Run to get job ID and execution results """ + self._validate_before_use() + self._ensure_client_apps_prepared() self._warn_potential_secrets_in_params() with self._temporary_exec_params(server_exec_params=server_exec_params, client_exec_params=client_exec_params): self.process_env(env) diff --git a/nvflare/recipe/utils.py b/nvflare/recipe/utils.py index 501533f7b7..b972f8ff63 100644 --- a/nvflare/recipe/utils.py +++ b/nvflare/recipe/utils.py @@ -21,10 +21,10 @@ from nvflare.apis.analytix import ANALYTIC_EVENT_TYPE from nvflare.apis.dxo import DataKind -from nvflare.apis.job_def import USER_SETTABLE_JOB_META_KEYS, JobMetaKey +from nvflare.apis.job_def import ALL_SITES, SERVER_SITE_NAME, USER_SETTABLE_JOB_META_KEYS, JobMetaKey from nvflare.fuel.utils.import_utils import optional_import from nvflare.fuel.utils.secret_utils import warn_on_potential_secrets, warn_on_unsupported_secret_refs -from nvflare.job_config.api import FedJob +from nvflare.job_config.api import FedJob, validate_target_name from nvflare.job_config.fed_job_config import FedJobConfig from nvflare.recipe.spec import Recipe @@ -265,6 +265,8 @@ def set_recipe_meta(recipe: Recipe, key: JobMetaKey, value: Any) -> None: def _validate_per_site_config_shape(config: Any) -> Dict[str, Dict]: if not isinstance(config, dict): raise TypeError(f"config must be a dict, got {type(config).__name__}") + if not config: + raise ValueError("config must not be empty") for site_name, site_config in config.items(): if not isinstance(site_name, str): @@ -278,17 +280,49 @@ def _validate_per_site_config_shape(config: Any) -> Dict[str, Dict]: def set_per_site_config(recipe: Recipe, config: Dict[str, Dict]) -> None: """Set site-keyed configuration on a recipe. - The helper only validates the generic shape: + Call this once, immediately after recipe construction and before adding + client customizations. The helper validates the generic shape: - top-level keys are site names - values are recipe-specific dictionaries + - the mapping is not empty Each recipe is responsible for validating and interpreting the fields inside - each site's dictionary. The execution environment still controls which - clients are present for a run. Per-site values become part of the generated - job definition and must never contain actual secret values; see - :mod:`nvflare.recipe.secrets`. + each site's dictionary. Supported recipes materialize client apps later, + before the first client customization or before export or execution. The + execution environment still controls which clients are present for a run. + Per-site values become part of the generated job definition and must never + contain actual secret values; see :mod:`nvflare.recipe.secrets`. """ - recipe.set_per_site_config(_validate_per_site_config_shape(config)) + recipe.set_per_site_config(config) + + +def _validate_per_site_targets(config: Dict[str, Dict], min_clients: int) -> None: + """Validate site targets and the minimum runnable site count.""" + reserved_targets = {SERVER_SITE_NAME, ALL_SITES} + for site_name in config: + validate_target_name(site_name) + if site_name in reserved_targets: + raise ValueError( + f"{site_name!r} is a reserved target name and cannot be used in per_site_config; " + f"reserved names: {sorted(reserved_targets)}" + ) + + if len(config) < min_clients: + raise ValueError( + f"per_site_config defines {len(config)} site(s), but min_clients={min_clients} requires at least " + f"{min_clients}" + ) + + +def _apply_legacy_constructor_config(recipe: Recipe, config: Dict[str, Dict]) -> None: + """Forward a deprecated constructor argument through the canonical setter.""" + warnings.warn( + f"{type(recipe).__name__}(per_site_config=...) is deprecated; construct the recipe without " + "per_site_config and call set_per_site_config(recipe, config) immediately after construction", + FutureWarning, + stacklevel=3, + ) + set_per_site_config(recipe, config) def _has_cross_site_eval_workflow(job: FedJob) -> bool: @@ -340,7 +374,7 @@ def add_experiment_tracking( To give sites different receiver configs (e.g. per-site tracking_uri), call this function once per site with that site's tracking_config and clients=[site]. Targeting specific clients requires the recipe's client apps to be per-site - (e.g. recipes constructed with the per_site_config constructor argument), and + (call set_per_site_config immediately after constructing a supported recipe), and each name must match an existing per-site client app; with the default all-clients topology or unknown site names, targeted placement raises ValueError. diff --git a/research/fedumm/job.py b/research/fedumm/job.py index c9a7352fe1..b2314f9c23 100644 --- a/research/fedumm/job.py +++ b/research/fedumm/job.py @@ -22,7 +22,7 @@ import argparse from nvflare.app_opt.pt.recipes.fedavg import FedAvgRecipe -from nvflare.recipe import SimEnv +from nvflare.recipe import SimEnv, set_per_site_config def _parse_args(): @@ -102,10 +102,10 @@ def main() -> None: min_clients=args.num_clients, num_rounds=args.num_rounds, train_script="client.py", - per_site_config=per_site_config, launch_external_process=False, key_metric="val_accuracy", ) + set_per_site_config(recipe, per_site_config) env = SimEnv( clients=client_names, diff --git a/tests/integration_test/slow/experiment_tracking_recipes_test.py b/tests/integration_test/slow/experiment_tracking_recipes_test.py index 885b0d2c93..42063372e0 100644 --- a/tests/integration_test/slow/experiment_tracking_recipes_test.py +++ b/tests/integration_test/slow/experiment_tracking_recipes_test.py @@ -37,7 +37,7 @@ import pytest from nvflare.app_opt.pt.recipes.fedavg import FedAvgRecipe -from nvflare.recipe import SimEnv +from nvflare.recipe import SimEnv, set_per_site_config from nvflare.recipe.utils import add_experiment_tracking INTEGRATION_TEST_ROOT = os.path.dirname(os.path.dirname(__file__)) @@ -122,8 +122,8 @@ def test_per_site_export_preserves_client_executors(self, tmp_path): num_rounds=1, model={"class_path": "model.SimpleNetwork", "args": {}}, train_script=self.client_script_path, - per_site_config=self._per_site_config(str(tmp_path / "data"), str(tmp_path)), ) + set_per_site_config(recipe, self._per_site_config(str(tmp_path / "data"), str(tmp_path))) self._add_model_to_apps(recipe) # Exercise the same mutation path as the runtime tests. @@ -143,8 +143,8 @@ def test_tensorboard_tracking_integration(self, cifar10_data_root): num_rounds=1, model={"class_path": "model.SimpleNetwork", "args": {}}, train_script=self.client_script_path, - per_site_config=self._per_site_config(cifar10_data_root, tmpdir), ) + set_per_site_config(recipe, self._per_site_config(cifar10_data_root, tmpdir)) self._add_model_to_apps(recipe) # Add TensorBoard tracking @@ -168,8 +168,8 @@ def test_mlflow_tracking_integration(self, cifar10_data_root): num_rounds=1, model={"class_path": "model.SimpleNetwork", "args": {}}, train_script=self.mlflow_client_script_path, - per_site_config=self._per_site_config(cifar10_data_root, tmpdir), ) + set_per_site_config(recipe, self._per_site_config(cifar10_data_root, tmpdir)) self._add_model_to_apps(recipe, self.mlflow_client_model_path) # Exercise the zero-config MLflow path: local storage and names derived from the recipe. @@ -206,8 +206,8 @@ def test_mlflow_client_tracking_defaults_integration(self, cifar10_data_root): num_rounds=1, model={"class_path": "model.SimpleNetwork", "args": {}}, train_script=self.mlflow_client_script_path, - per_site_config=self._per_site_config(cifar10_data_root, tmpdir), ) + set_per_site_config(recipe, self._per_site_config(cifar10_data_root, tmpdir)) self._add_model_to_apps(recipe, self.mlflow_client_model_path) add_experiment_tracking(recipe, "mlflow", client_side=True, server_side=False) diff --git a/tests/integration_test/slow/xgb_histogram_recipe_test.py b/tests/integration_test/slow/xgb_histogram_recipe_test.py index c629115a59..8abe20c620 100644 --- a/tests/integration_test/slow/xgb_histogram_recipe_test.py +++ b/tests/integration_test/slow/xgb_histogram_recipe_test.py @@ -42,7 +42,7 @@ from nvflare.app_opt.xgboost.data_loader import XGBDataLoader from nvflare.app_opt.xgboost.recipes import XGBHorizontalRecipe -from nvflare.recipe import SimEnv +from nvflare.recipe import SimEnv, set_per_site_config class MockXGBDataLoader(XGBDataLoader): @@ -99,8 +99,8 @@ def test_histogram_algorithm(self): "objective": "binary:logistic", "eval_metric": "auc", }, - per_site_config=per_site_config, ) + set_per_site_config(recipe, per_site_config) # Run and verify completion run = recipe.execute(env) @@ -127,8 +127,8 @@ def test_custom_xgb_params(self): min_clients=2, num_rounds=1, xgb_params=custom_params, - per_site_config=per_site_config, ) + set_per_site_config(recipe, per_site_config) # Verify params are stored assert recipe.xgb_params == custom_params @@ -147,8 +147,8 @@ def test_multiple_clients(self): name="test_multi_client", min_clients=num_clients, num_rounds=1, - per_site_config=per_site_config, ) + set_per_site_config(recipe, per_site_config) run = recipe.execute(env) assert run.get_result() is not None diff --git a/tests/integration_test/slow/xgb_vertical_recipe_test.py b/tests/integration_test/slow/xgb_vertical_recipe_test.py index cb35e15d18..c1c2509d25 100644 --- a/tests/integration_test/slow/xgb_vertical_recipe_test.py +++ b/tests/integration_test/slow/xgb_vertical_recipe_test.py @@ -42,7 +42,7 @@ from nvflare.app_opt.xgboost.data_loader import XGBDataLoader from nvflare.app_opt.xgboost.recipes import XGBVerticalRecipe -from nvflare.recipe import SimEnv +from nvflare.recipe import SimEnv, set_per_site_config class MockVerticalDataLoader(XGBDataLoader): @@ -114,8 +114,8 @@ def test_vertical_basic(self): "objective": "binary:logistic", "eval_metric": "auc", }, - per_site_config=per_site_config, ) + set_per_site_config(recipe, per_site_config) # Run and verify completion run = recipe.execute(env) @@ -130,8 +130,8 @@ def test_label_owner_validation(self): min_clients=2, num_rounds=1, label_owner="site-1", # Valid - per_site_config=_make_vertical_per_site_config(), ) + set_per_site_config(recipe, _make_vertical_per_site_config()) assert recipe.label_owner == "site-1" assert recipe.client_ranks["site-1"] == 0 @@ -142,53 +142,52 @@ def test_label_owner_validation(self): min_clients=2, num_rounds=1, label_owner="client1", # Invalid format - per_site_config=_make_vertical_per_site_config(), ) # Label owner must be rank 0 for vertical XGBoost. + recipe = XGBVerticalRecipe( + name="test_invalid_rank", + min_clients=2, + num_rounds=1, + label_owner="site-2", + client_ranks={"site-1": 0, "site-2": 1}, + ) with pytest.raises(ValueError, match="label_owner must be assigned rank 0"): - XGBVerticalRecipe( - name="test_invalid_rank", - min_clients=2, - num_rounds=1, - label_owner="site-2", - client_ranks={"site-1": 0, "site-2": 1}, - per_site_config=_make_vertical_per_site_config(label_owner="site-2"), - ) + set_per_site_config(recipe, _make_vertical_per_site_config(label_owner="site-2")) def test_client_ranks_validation(self): """Test that client_ranks validation catches malformed rank mappings.""" per_site_config = _make_vertical_per_site_config(num_clients=3) + recipe = XGBVerticalRecipe( + name="test_non_integer_ranks", + min_clients=3, + num_rounds=1, + label_owner="site-1", + client_ranks={"site-1": 0, "site-2": "1", "site-3": 2}, + ) with pytest.raises(ValueError, match="client_ranks values must be integers"): - XGBVerticalRecipe( - name="test_non_integer_ranks", - min_clients=3, - num_rounds=1, - label_owner="site-1", - client_ranks={"site-1": 0, "site-2": "1", "site-3": 2}, - per_site_config=per_site_config, - ) + set_per_site_config(recipe, per_site_config) + recipe = XGBVerticalRecipe( + name="test_duplicate_ranks", + min_clients=3, + num_rounds=1, + label_owner="site-1", + client_ranks={"site-1": 0, "site-2": 1, "site-3": 1}, + ) with pytest.raises(ValueError, match="client_ranks values must be unique and consecutive"): - XGBVerticalRecipe( - name="test_duplicate_ranks", - min_clients=3, - num_rounds=1, - label_owner="site-1", - client_ranks={"site-1": 0, "site-2": 1, "site-3": 1}, - per_site_config=per_site_config, - ) + set_per_site_config(recipe, per_site_config) + recipe = XGBVerticalRecipe( + name="test_non_consecutive_ranks", + min_clients=3, + num_rounds=1, + label_owner="site-1", + client_ranks={"site-1": 0, "site-2": 1, "site-3": 3}, + ) with pytest.raises(ValueError, match="client_ranks values must be unique and consecutive"): - XGBVerticalRecipe( - name="test_non_consecutive_ranks", - min_clients=3, - num_rounds=1, - label_owner="site-1", - client_ranks={"site-1": 0, "site-2": 1, "site-3": 3}, - per_site_config=per_site_config, - ) + set_per_site_config(recipe, per_site_config) def test_custom_xgb_params(self): """Test that custom XGBoost parameters are accepted.""" @@ -207,8 +206,8 @@ def test_custom_xgb_params(self): num_rounds=1, label_owner="site-1", xgb_params=custom_params, - per_site_config=_make_vertical_per_site_config(), ) + set_per_site_config(recipe, _make_vertical_per_site_config()) # Verify params are stored assert recipe.xgb_params == custom_params @@ -227,8 +226,8 @@ def test_multiple_clients_vertical(self): min_clients=num_clients, num_rounds=1, label_owner="site-2", # site-2 has labels - per_site_config=per_site_config, ) + set_per_site_config(recipe, per_site_config) assert recipe.client_ranks["site-2"] == 0 run = recipe.execute(env) @@ -243,8 +242,8 @@ def test_in_process_parameter(self): num_rounds=1, label_owner="site-1", in_process=True, # Default - per_site_config=_make_vertical_per_site_config(), ) + set_per_site_config(recipe, _make_vertical_per_site_config()) assert recipe.in_process is True recipe2 = XGBVerticalRecipe( @@ -253,8 +252,8 @@ def test_in_process_parameter(self): num_rounds=1, label_owner="site-1", in_process=False, - per_site_config=_make_vertical_per_site_config(), ) + set_per_site_config(recipe2, _make_vertical_per_site_config()) assert recipe2.in_process is False def test_model_file_name_parameter(self): @@ -266,6 +265,6 @@ def test_model_file_name_parameter(self): num_rounds=1, label_owner="site-1", model_file_name=custom_name, - per_site_config=_make_vertical_per_site_config(), ) + set_per_site_config(recipe, _make_vertical_per_site_config()) assert recipe.model_file_name == custom_name diff --git a/tests/unit_test/app_opt/pt/recipes/fed_eval_recipe_test.py b/tests/unit_test/app_opt/pt/recipes/fed_eval_recipe_test.py index b1dd92b2b2..53c2c5f4ca 100644 --- a/tests/unit_test/app_opt/pt/recipes/fed_eval_recipe_test.py +++ b/tests/unit_test/app_opt/pt/recipes/fed_eval_recipe_test.py @@ -19,9 +19,11 @@ import torch.nn as nn from pydantic import ValidationError +from nvflare.apis.job_def import ALL_SITES from nvflare.app_opt.pt.recipes.fedeval import FedEvalRecipe from nvflare.client.config import ExchangeFormat from nvflare.fuel.utils.secret_utils import PotentialSecretWarning, UnsupportedSecretRefWarning +from nvflare.recipe import set_per_site_config class SimpleTestModel(nn.Module): @@ -74,6 +76,10 @@ def assert_recipe_basics(recipe, expected_name, expected_params): assert recipe._job.name == expected_name +def get_client_executor(recipe, site_name): + return recipe._job._deploy_map[site_name].app_config.executors[0].executor + + class TestFedEvalRecipe: """Test cases for FedEvalRecipe class.""" @@ -91,16 +97,20 @@ def test_external_command_secret_ref_is_supported(self, mock_file_system, base_r model, _ = simple_model with warnings.catch_warnings(): warnings.simplefilter("error", UnsupportedSecretRefWarning) - FedEvalRecipe( + recipe = FedEvalRecipe( name="command_secret_ref", model=model, - per_site_config={ + **base_recipe_params, + ) + set_per_site_config( + recipe, + { "site-1": { "launch_external_process": True, "command": "env API_TOKEN=${secret:API_TOKEN} python3 -u", - } + }, + "site-2": {}, }, - **base_recipe_params, ) def test_basic_initialization(self, mock_file_system, base_recipe_params, simple_model): @@ -117,6 +127,26 @@ def test_basic_initialization(self, mock_file_system, base_recipe_params, simple assert recipe.validation_timeout == 6000 assert recipe.per_site_config is None + def test_set_per_site_config_prepares_site_runners_before_client_customization( + self, mock_file_system, base_recipe_params, simple_model + ): + model, _ = simple_model + recipe = FedEvalRecipe(name="test_helper_per_site", model=model, **base_recipe_params) + config = {"site-1": {"eval_args": "--batch_size 8"}, "site-2": {}} + + assert recipe._job.clients == [] + set_per_site_config(recipe, config) + + assert recipe.configured_sites() == ["site-1", "site-2"] + assert recipe._job.clients == [] + + recipe.add_client_config({"configured": True}) + + assert recipe._job.clients == ["site-1", "site-2"] + assert ALL_SITES not in recipe._job._deploy_map + assert get_client_executor(recipe, "site-1")._task_script_args == "--batch_size 8" + assert get_client_executor(recipe, "site-2")._task_script_args == "--batch_size 32" + def test_default_job_name(self, mock_file_system, base_recipe_params, simple_model): """Test FedEvalRecipe with default job name.""" model, _ = simple_model @@ -219,12 +249,13 @@ def test_per_site_config(self, mock_file_system, base_recipe_params, simple_mode }, } - recipe = FedEvalRecipe( - name="test_per_site", - model=model, - per_site_config=per_site_config, - **base_recipe_params, - ) + with pytest.warns(FutureWarning, match="set_per_site_config"): + recipe = FedEvalRecipe( + name="test_per_site", + model=model, + per_site_config=per_site_config, + **base_recipe_params, + ) assert recipe.per_site_config == per_site_config @@ -235,14 +266,11 @@ def test_per_site_config_partial_overrides(self, mock_file_system, base_recipe_p "site-1": { "eval_args": "--batch_size 16", }, + "site-2": {}, } - recipe = FedEvalRecipe( - name="test_partial_override", - model=model, - per_site_config=per_site_config, - **base_recipe_params, - ) + recipe = FedEvalRecipe(name="test_partial_override", model=model, **base_recipe_params) + set_per_site_config(recipe, per_site_config) # Check that default values are stored assert recipe.eval_script == "mock_eval_script.py" @@ -253,6 +281,15 @@ def test_per_site_config_partial_overrides(self, mock_file_system, base_recipe_p class TestFedEvalRecipeValidation: """Test FedEvalRecipe input validation.""" + def test_per_site_config_requires_at_least_min_clients(self, mock_file_system, base_recipe_params, simple_model): + model, _ = simple_model + recipe = FedEvalRecipe(name="test_per_site_client_count", model=model, **base_recipe_params) + + with pytest.raises(ValueError, match=r"defines 1 site.*min_clients=2"): + set_per_site_config(recipe, {"site-1": {}}) + + assert recipe._job.clients == [] + def test_invalid_eval_ckpt_raises_error(self, simple_model): """Test that relative eval_ckpt raises when path does not exist locally. diff --git a/tests/unit_test/app_opt/sklearn/sklearn_recipe_test.py b/tests/unit_test/app_opt/sklearn/sklearn_recipe_test.py index 4f8d8e115a..07c7da6238 100644 --- a/tests/unit_test/app_opt/sklearn/sklearn_recipe_test.py +++ b/tests/unit_test/app_opt/sklearn/sklearn_recipe_test.py @@ -121,9 +121,11 @@ def test_with_per_site_config(self, mock_file_system, base_recipe_params): recipe = SklearnFedAvgRecipe( name="test_sklearn_per_site", model_params={"n_classes": 2}, - per_site_config=per_site_config, **base_recipe_params, ) + from nvflare.recipe import set_per_site_config + + set_per_site_config(recipe, per_site_config) assert recipe.per_site_config == per_site_config diff --git a/tests/unit_test/app_opt/xgboost/xgboost_recipe_test.py b/tests/unit_test/app_opt/xgboost/xgboost_recipe_test.py index 42dbaf1469..456a5d2ae9 100644 --- a/tests/unit_test/app_opt/xgboost/xgboost_recipe_test.py +++ b/tests/unit_test/app_opt/xgboost/xgboost_recipe_test.py @@ -16,9 +16,15 @@ from nvflare.apis.job_def import SERVER_SITE_NAME from nvflare.app_common.widgets.metrics_artifact_writer import MetricsArtifactWriter +from nvflare.recipe import set_per_site_config pytest.importorskip("xgboost") +xgb_recipes = pytest.importorskip("nvflare.app_opt.xgboost.recipes") +XGBBaggingRecipe = xgb_recipes.XGBBaggingRecipe +XGBHorizontalRecipe = xgb_recipes.XGBHorizontalRecipe +XGBVerticalRecipe = xgb_recipes.XGBVerticalRecipe + class DummyDataLoader: pass @@ -36,31 +42,92 @@ def _get_metrics_writer(recipe): return server_app.app_config.components.get("metrics_artifact_writer") -def test_xgb_bagging_recipe_configures_metrics_artifact_writer(): - from nvflare.app_opt.xgboost.recipes import XGBBaggingRecipe +def _get_server_controller(recipe, controller_id): + server_app = recipe._job._deploy_map[SERVER_SITE_NAME] + return next( + (workflow.controller for workflow in server_app.app_config.workflows if workflow.id == controller_id), + None, + ) + - recipe = XGBBaggingRecipe(name="xgb_bagging", min_clients=2, per_site_config=_per_site_config()) +@pytest.mark.parametrize( + "recipe", + [ + pytest.param(XGBBaggingRecipe(name="xgb_bagging", min_clients=2), id="bagging"), + pytest.param(XGBHorizontalRecipe(name="xgb_horizontal", min_clients=2, num_rounds=1), id="horizontal"), + pytest.param( + XGBVerticalRecipe(name="xgb_vertical", min_clients=2, num_rounds=1, label_owner="site-1"), + id="vertical", + ), + ], +) +def test_xgb_recipes_apply_helper_config(recipe, tmp_path): + config = _per_site_config() assert isinstance(_get_metrics_writer(recipe), MetricsArtifactWriter) + with pytest.raises(RuntimeError, match="requires set_per_site_config"): + recipe.export(str(tmp_path)) + set_per_site_config(recipe, config) -def test_xgb_horizontal_recipe_configures_metrics_artifact_writer(): - from nvflare.app_opt.xgboost.recipes import XGBHorizontalRecipe + assert recipe.configured_sites() == ["site-1", "site-2"] + assert recipe._job.clients == [] - recipe = XGBHorizontalRecipe(name="xgb_horizontal", min_clients=2, num_rounds=1, per_site_config=_per_site_config()) + recipe.add_client_config({"configured": True}) - assert isinstance(_get_metrics_writer(recipe), MetricsArtifactWriter) + assert recipe._job.clients == ["site-1", "site-2"] + for site_name, site_config in config.items(): + components = recipe._job._deploy_map[site_name].app_config.components + assert components[recipe.data_loader_id] is site_config["data_loader"] + assert _get_server_controller(recipe, "xgb_controller") is not None + recipe._validate_before_use() + + +def test_xgb_legacy_constructor_config_delegates_to_helper(): + config = _per_site_config() + + with pytest.warns(FutureWarning, match="set_per_site_config"): + recipe = XGBBaggingRecipe(name="xgb_legacy", min_clients=2, per_site_config=config) + assert recipe.configured_sites() == ["site-1", "site-2"] + assert recipe._job.clients == [] + recipe._ensure_client_apps_prepared() + assert recipe._job.clients == ["site-1", "site-2"] -def test_xgb_vertical_recipe_configures_metrics_artifact_writer(): - from nvflare.app_opt.xgboost.recipes import XGBVerticalRecipe +def test_xgb_per_site_config_validates_client_count_and_data_loader(): + recipe = XGBHorizontalRecipe(name="xgb_validation", min_clients=2, num_rounds=1) + + with pytest.raises(ValueError, match=r"defines 1 site.*min_clients=2"): + set_per_site_config(recipe, {"site-1": {"data_loader": DummyDataLoader()}}) + with pytest.raises(ValueError, match="site-2.*data_loader"): + set_per_site_config( + recipe, + { + "site-1": {"data_loader": DummyDataLoader()}, + "site-2": {}, + }, + ) + + assert recipe._job.clients == [] + assert recipe.configured_sites() == [] + + +def test_xgb_vertical_resolves_label_owner_rank_when_helper_is_applied(): recipe = XGBVerticalRecipe( - name="xgb_vertical", + name="xgb_vertical_ranks", min_clients=2, num_rounds=1, - label_owner="site-1", - per_site_config=_per_site_config(), + label_owner="site-2", ) - assert isinstance(_get_metrics_writer(recipe), MetricsArtifactWriter) + set_per_site_config(recipe, _per_site_config()) + + assert recipe.client_ranks == {"site-2": 0, "site-1": 1} + assert _get_server_controller(recipe, "xgb_controller") is None + + recipe._ensure_client_apps_prepared() + + controller = _get_server_controller(recipe, "xgb_controller") + assert controller is not None + assert controller.client_ranks == recipe.client_ranks diff --git a/tests/unit_test/recipe/fedavg_recipe_test.py b/tests/unit_test/recipe/fedavg_recipe_test.py index 268ce617e4..c09c6ee82c 100644 --- a/tests/unit_test/recipe/fedavg_recipe_test.py +++ b/tests/unit_test/recipe/fedavg_recipe_test.py @@ -39,6 +39,7 @@ from nvflare.fuel.utils.class_utils import instantiate_class from nvflare.fuel.utils.secret_utils import UnsupportedSecretRefWarning from nvflare.job_config.base_fed_job import BaseFedJob +from nvflare.recipe import set_per_site_config from nvflare.recipe.fedavg import FedAvgRecipe as BaseFedAvgRecipe @@ -183,6 +184,11 @@ def get_server_controller(recipe): return server_app.app_config.workflows[0].controller +def get_client_executor(recipe, site_name): + client_app = recipe._job._deploy_map[site_name] + return client_app.app_config.executors[0].executor + + def _get_train_executor_config(client_config): for executor_entry in client_config.get("executors", []): executor = executor_entry["executor"] @@ -234,16 +240,20 @@ def test_class_docstrings_are_preserved(self): def test_external_command_secret_ref_is_supported(self, mock_file_system, base_recipe_params, simple_model): with warnings.catch_warnings(): warnings.simplefilter("error", UnsupportedSecretRefWarning) - FedAvgRecipe( + recipe = FedAvgRecipe( name="command_secret_ref", model=simple_model, - per_site_config={ + **base_recipe_params, + ) + set_per_site_config( + recipe, + { "site-1": { "launch_external_process": True, "command": "env API_TOKEN=${secret:API_TOKEN} python3 -u", - } + }, + "site-2": {}, }, - **base_recipe_params, ) """Test cases for FedAvgRecipe class.""" @@ -257,6 +267,75 @@ def test_default_aggregator_initialization(self, mock_file_system, base_recipe_p # When no aggregator is passed, built-in weighted averaging is used assert recipe.aggregator is None + def test_set_per_site_config_prepares_site_runners_before_client_customization( + self, mock_file_system, base_recipe_params, simple_model + ): + recipe = FedAvgRecipe(name="test_helper_per_site", model=simple_model, **base_recipe_params) + config = { + "site-1": {"train_args": "--epochs 1"}, + "site-2": {}, + } + + assert recipe._job.clients == [] + set_per_site_config(recipe, config) + + assert recipe.configured_sites() == ["site-1", "site-2"] + assert recipe._job.clients == [] + + recipe.add_client_config({"configured": True}) + + assert recipe._job.clients == ["site-1", "site-2"] + assert ALL_SITES not in recipe._job._deploy_map + assert get_client_executor(recipe, "site-1")._task_script_args == "--epochs 1" + assert get_client_executor(recipe, "site-2")._task_script_args == "--epochs 10" + + def test_set_per_site_config_snapshots_overrides_before_deferred_preparation( + self, mock_file_system, base_recipe_params, simple_model + ): + recipe = FedAvgRecipe(name="test_helper_snapshot", model=simple_model, **base_recipe_params) + config = {"site-1": {"train_args": "--epochs 1"}, "site-2": {}} + + set_per_site_config(recipe, config) + config["site-1"]["train_args"] = "--epochs 99" + + recipe._ensure_client_apps_prepared() + + assert get_client_executor(recipe, "site-1")._task_script_args == "--epochs 1" + + def test_legacy_constructor_config_delegates_to_helper(self, mock_file_system, base_recipe_params, simple_model): + config = {"site-1": {"train_args": "--epochs 1"}, "site-2": {}} + + with pytest.warns(FutureWarning, match="set_per_site_config"): + recipe = FedAvgRecipe( + name="test_legacy_per_site", + model=simple_model, + per_site_config=config, + **base_recipe_params, + ) + + assert recipe.configured_sites() == ["site-1", "site-2"] + assert recipe._job.clients == [] + recipe._ensure_client_apps_prepared() + assert recipe._job.clients == ["site-1", "site-2"] + assert get_client_executor(recipe, "site-1")._task_script_args == "--epochs 1" + + def test_failed_per_site_config_leaves_topology_unprepared_and_can_retry( + self, mock_file_system, base_recipe_params, simple_model + ): + recipe = FedAvgRecipe(name="test_per_site_rollback", model=simple_model, **base_recipe_params) + + with pytest.raises(ValueError, match="Framework invalid unsupported"): + set_per_site_config(recipe, {"site-1": {}, "site-2": {"framework": "invalid"}}) + + assert recipe._job.clients == [] + assert recipe.configured_sites() == [] + assert recipe.per_site_config is None + + set_per_site_config(recipe, {"site-1": {}, "site-2": {}}) + assert recipe._job.clients == [] + recipe._ensure_client_apps_prepared() + assert recipe._job.clients == ["site-1", "site-2"] + def test_tensor_disk_offload_warns_when_server_format_is_not_pytorch( self, mock_file_system, base_recipe_params, simple_model ): @@ -523,16 +602,38 @@ def test_numpy_recipe_with_per_site_config(self, mock_file_system): "site-1": {"train_args": "--data /path/to/site1"}, "site-2": {"train_args": "--data /path/to/site2"}, } + with pytest.warns(FutureWarning, match="set_per_site_config"): + recipe = NumpyFedAvgRecipe( + name="test_numpy_per_site", + model=[1.0, 2.0], + min_clients=2, + num_rounds=3, + train_script="client.py", + per_site_config=per_site_config, + ) + + assert recipe.per_site_config == per_site_config + + def test_numpy_helper_config_preserves_numpy_runner_exchange_format(self, mock_file_system): + from nvflare.client.config import ExchangeFormat + from nvflare.fuel.utils.constants import FrameworkType + recipe = NumpyFedAvgRecipe( - name="test_numpy_per_site", + name="test_numpy_helper_format", model=[1.0, 2.0], min_clients=2, - num_rounds=3, train_script="client.py", - per_site_config=per_site_config, ) + set_per_site_config(recipe, {"site-1": {}, "site-2": {}}) - assert recipe.per_site_config == per_site_config + # NumPy recipes identify as RAW to Recipe CSE utilities, but their + # training runners must continue exchanging NumPy parameters. + assert recipe.framework == FrameworkType.RAW + recipe._ensure_client_apps_prepared() + + for site in ["site-1", "site-2"]: + executor = get_client_executor(recipe, site) + assert executor._params_exchange_format == ExchangeFormat.NUMPY def test_numpy_cse_export_preserves_per_site_training_apps(self, tmp_path): """Adding CSE must retain each per-site training executor alongside NPValidator.""" @@ -546,8 +647,8 @@ def test_numpy_cse_export_preserves_per_site_training_apps(self, tmp_path): model=[1.0, 2.0], min_clients=2, train_script=str(train_script), - per_site_config={site: {"train_args": f"--site {site}"} for site in sites}, ) + set_per_site_config(recipe, {site: {"train_args": f"--site {site}"} for site in sites}) add_cross_site_evaluation(recipe) export_dir = tmp_path / "export" @@ -725,25 +826,57 @@ def test_invalid_aggregator_type_raises_validation_error(self, mock_file_system, def test_per_site_config_rejects_reserved_server_target(self, mock_file_system, base_recipe_params, simple_model): """Reserved target 'server' must not be allowed in per_site_config.""" + recipe = FedAvgRecipe(name="test_reserved_server_target", model=simple_model, **base_recipe_params) + with pytest.raises(ValueError, match="reserved target name"): - FedAvgRecipe( - name="test_reserved_server_target", - model=simple_model, - per_site_config={"server": {}}, - **base_recipe_params, - ) + set_per_site_config(recipe, {"server": {}}) def test_per_site_config_rejects_reserved_all_sites_target( self, mock_file_system, base_recipe_params, simple_model ): """Reserved target '@ALL' must not be allowed in per_site_config.""" + recipe = FedAvgRecipe(name="test_reserved_all_sites_target", model=simple_model, **base_recipe_params) + with pytest.raises(ValueError, match="reserved target name"): - FedAvgRecipe( - name="test_reserved_all_sites_target", - model=simple_model, - per_site_config={ALL_SITES: {}}, - **base_recipe_params, - ) + set_per_site_config(recipe, {ALL_SITES: {}}) + + @pytest.mark.parametrize( + ("site_name", "match"), + [ + ("", "valid target name"), + ("site/1", "invalid character"), + ("site@1", "invalid character"), + ], + ) + def test_per_site_config_rejects_invalid_target_name( + self, mock_file_system, base_recipe_params, simple_model, site_name, match + ): + recipe = FedAvgRecipe(name="test_invalid_target_name", model=simple_model, **base_recipe_params) + + with pytest.raises(ValueError, match=match): + set_per_site_config(recipe, {site_name: {}, "site-2": {}}) + + assert recipe.configured_sites() == [] + assert recipe._job.clients == [] + + def test_per_site_config_requires_at_least_min_clients(self, mock_file_system, base_recipe_params, simple_model): + recipe = FedAvgRecipe(name="test_per_site_client_count", model=simple_model, **base_recipe_params) + + with pytest.raises(ValueError, match=r"defines 1 site.*min_clients=2"): + set_per_site_config(recipe, {"site-1": {}}) + + assert recipe._job.clients == [] + + def test_per_site_config_rejects_after_export(self, tmp_path, base_recipe_params, simple_model): + train_script = tmp_path / "train.py" + train_script.write_text("print('training')\n") + params = dict(base_recipe_params, train_script=str(train_script)) + recipe = FedAvgRecipe(name="test_deployed_per_site", model=simple_model, **params) + recipe.export(str(tmp_path)) + assert recipe._job.clients == [ALL_SITES] + + with pytest.raises(RuntimeError, match="immediately after recipe construction"): + set_per_site_config(recipe, {"site-1": {}, "site-2": {}}) def test_per_site_empty_command_override_is_preserved(self, mock_file_system, base_recipe_params, simple_model): """Falsy per-site override values (e.g. command='') must not be replaced by defaults.""" @@ -751,9 +884,10 @@ def test_per_site_empty_command_override_is_preserved(self, mock_file_system, ba name="test_empty_command_override", model=simple_model, launch_external_process=True, - per_site_config={"site-1": {"command": ""}}, **base_recipe_params, ) + set_per_site_config(recipe, {"site-1": {"command": ""}, "site-2": {}}) + recipe._ensure_client_apps_prepared() site_app = recipe._job._deploy_map.get("site-1") assert site_app is not None diff --git a/tests/unit_test/recipe/recipe_secrets_test.py b/tests/unit_test/recipe/recipe_secrets_test.py index 670af3148c..9dbebd6635 100644 --- a/tests/unit_test/recipe/recipe_secrets_test.py +++ b/tests/unit_test/recipe/recipe_secrets_test.py @@ -89,12 +89,14 @@ def test_clean_train_args_no_warning(self, make_recipe): assert _no_secret_warnings(record) def test_per_site_config_with_password_warns(self, make_recipe): - recipe = make_recipe( - per_site_config={ - "site-1": {"train_args": "--password hunter22x"}, - "site-2": {"train_args": "--epochs 5"}, - } - ) + recipe = make_recipe() + with pytest.warns(PotentialSecretWarning): + recipe.set_per_site_config( + { + "site-1": {"train_args": "--password hunter22x"}, + "site-2": {"train_args": "--epochs 5"}, + } + ) with pytest.warns(PotentialSecretWarning) as record: recipe._warn_potential_secrets_in_params() messages = [str(w.message) for w in record] @@ -104,7 +106,7 @@ def test_per_site_config_with_password_warns(self, make_recipe): def test_set_per_site_config_warns(self, make_recipe): recipe = make_recipe() with pytest.warns(PotentialSecretWarning): - recipe.set_per_site_config({"site-1": {"api_key": "abcd1234efgh"}}) + recipe.set_per_site_config({"site-1": {"api_key": "abcd1234efgh"}, "site-2": {}}) def test_add_client_config_warns(self, make_recipe): recipe = make_recipe() diff --git a/tests/unit_test/recipe/spec_test.py b/tests/unit_test/recipe/spec_test.py index f0b2574f66..a87ed9ae02 100644 --- a/tests/unit_test/recipe/spec_test.py +++ b/tests/unit_test/recipe/spec_test.py @@ -253,8 +253,8 @@ def test_add_client_file_preserves_per_site_clients_without_all_sites(self, temp min_clients=2, train_script=temp_script, model={"class_path": "model.DummyModel", "args": {}}, - per_site_config={"site-1": {}, "site-2": {}}, ) + set_per_site_config(recipe, {"site-1": {}, "site-2": {}}) recipe.add_client_file(temp_script) @@ -276,8 +276,8 @@ def test_add_client_file_with_specific_clients_only_updates_selected_sites(self, min_clients=2, train_script=temp_script, model={"class_path": "model.DummyModel", "args": {}}, - per_site_config={"site-1": {}, "site-2": {}, "site-3": {}}, ) + set_per_site_config(recipe, {"site-1": {}, "site-2": {}, "site-3": {}}) with tempfile.NamedTemporaryFile(mode="w", suffix=".txt", delete=False) as f: f.write("targeted file for site-2 only") @@ -749,7 +749,34 @@ def _apply_per_site_config(self, config): assert recipe.configured_sites() == ["site-1", "site-2"] assert recipe.applied_config == config assert recipe.applied_config is not config - assert recipe.applied_config["site-1"] is config["site-1"] + assert recipe.applied_config["site-1"] is not config["site-1"] + + def test_set_per_site_config_snapshots_site_dicts_but_retains_value_objects(self): + from nvflare.recipe.spec import Recipe + + class RecordingRecipe(Recipe): + def __init__(self): + super().__init__(FedJob(name="test_per_site_config_snapshot", min_clients=1)) + self.per_site_config = None + + def _apply_per_site_config(self, config): + self.per_site_config = config + + data_loader = object() + config = { + "site-1": {"train_args": "--epochs 1", "data_loader": data_loader}, + "site-2": {}, + } + recipe = RecordingRecipe() + + set_per_site_config(recipe, config) + config["site-1"]["train_args"] = "--epochs 99" + config["site-1"]["new_value"] = True + config["site-3"] = {} + + assert recipe.configured_sites() == ["site-1", "site-2"] + assert recipe.per_site_config["site-1"] == {"train_args": "--epochs 1", "data_loader": data_loader} + assert recipe.per_site_config["site-1"]["data_loader"] is data_loader def test_set_per_site_config_hook_mutation_does_not_change_configured_sites(self): from nvflare.recipe.spec import Recipe @@ -783,6 +810,35 @@ def __init__(self): assert recipe.configured_sites() == ["site-1", "site-2"] assert recipe._job._deploy_map == {} + def test_client_apps_are_prepared_once_before_client_customization(self): + from nvflare.recipe.spec import Recipe + + class PreparingRecipe(Recipe): + def __init__(self): + super().__init__(FedJob(name="test_prepare_per_site_apps", min_clients=1)) + self.prepare_calls = 0 + + def _prepare_client_apps(self): + self.prepare_calls += 1 + for site_name in self.configured_sites(): + self._job.to({"executor_standin": True}, site_name) + + recipe = PreparingRecipe() + set_per_site_config(recipe, {"site-1": {}, "site-2": {}}) + + assert recipe.prepare_calls == 0 + assert recipe._job.clients == [] + + recipe.add_client_config({"timeout": 600}) + recipe.add_client_config({"streaming_chunk_size": 1024}) + + assert recipe.prepare_calls == 1 + assert recipe._job.clients == ["site-1", "site-2"] + for site_name in recipe.configured_sites(): + params = recipe._job._deploy_map[site_name].app_config.additional_params + assert params["timeout"] == 600 + assert params["streaming_chunk_size"] == 1024 + def test_configured_sites_prefers_helper_config_over_legacy_constructor_config(self): from nvflare.recipe.spec import Recipe @@ -798,7 +854,7 @@ def __init__(self): assert recipe.configured_sites() == ["helper-1"] - def test_empty_helper_config_still_overrides_legacy_constructor_config(self): + def test_empty_helper_config_is_rejected_without_overriding_legacy_config(self): from nvflare.recipe.spec import Recipe class LegacyRecipe(Recipe): @@ -808,10 +864,47 @@ def __init__(self): recipe = LegacyRecipe() - set_per_site_config(recipe, {}) + with pytest.raises(ValueError, match="must not be empty"): + set_per_site_config(recipe, {}) + + assert recipe.configured_sites() == ["legacy-1"] + + def test_failed_recipe_hook_does_not_record_helper_config(self): + from nvflare.recipe.spec import Recipe + + class FailingRecipe(Recipe): + def __init__(self): + super().__init__(FedJob(name="test_failed_per_site_hook", min_clients=1)) + + def _apply_per_site_config(self, config): + raise ValueError("invalid recipe-specific value") + + recipe = FailingRecipe() + + with pytest.raises(ValueError, match="invalid recipe-specific value"): + set_per_site_config(recipe, {"site-1": {}}) assert recipe.configured_sites() == [] + def test_per_site_config_can_only_be_applied_once(self): + from nvflare.recipe.spec import Recipe + + recipe = Recipe(FedJob(name="test_repeated_per_site_config", min_clients=1)) + set_per_site_config(recipe, {"site-1": {}}) + + with pytest.raises(RuntimeError, match="already been applied"): + set_per_site_config(recipe, {"site-2": {}}) + + def test_per_site_config_must_precede_client_customization(self): + from nvflare.recipe.spec import Recipe + + recipe = Recipe(FedJob(name="test_late_per_site_config", min_clients=1)) + recipe._job.to_clients({"executor_standin": True}) + recipe.add_client_config({"timeout": 600}) + + with pytest.raises(RuntimeError, match="immediately after recipe construction"): + set_per_site_config(recipe, {"site-1": {}}) + def test_configured_sites_does_not_infer_from_job_meta(self): from nvflare.recipe.spec import Recipe @@ -851,6 +944,23 @@ def __init__(self): with pytest.raises(TypeError, match=match): set_per_site_config(BasicRecipe(), config) + @pytest.mark.parametrize( + "config, exception, match", + [ + ("not-a-dict", TypeError, "per-site config must be a dict"), + ({}, ValueError, "per-site config must not be empty"), + ({1: {}}, TypeError, "per-site config key must be a str"), + ({"site-1": "not-a-dict"}, TypeError, "per-site config for site 'site-1' must be a dict"), + ], + ) + def test_recipe_method_validates_generic_shape(self, config, exception, match): + from nvflare.recipe.spec import Recipe + + recipe = Recipe(FedJob(name="test_direct_per_site_validation", min_clients=1)) + + with pytest.raises(exception, match=match): + recipe.set_per_site_config(config) + class TestClientPlacementHardening: """Test _add_to_client_apps validation through the public clients=-taking helpers. diff --git a/tests/unit_test/recipe/utils_test.py b/tests/unit_test/recipe/utils_test.py index 06f488498e..2c5c4185e5 100644 --- a/tests/unit_test/recipe/utils_test.py +++ b/tests/unit_test/recipe/utils_test.py @@ -318,6 +318,16 @@ def test_set_per_site_config_importable_from_recipe(self): assert callable(set_per_site_config) + def test_set_per_site_config_delegates_validation_to_recipe(self): + from nvflare.recipe import set_per_site_config + + recipe = MagicMock(spec=Recipe) + config = {"site-1": {}} + + set_per_site_config(recipe, config) + + recipe.set_per_site_config.assert_called_once_with(config) + def test_set_recipe_meta_importable_from_recipe(self): """set_recipe_meta must be importable from the top-level nvflare.recipe package.""" from nvflare.recipe import set_recipe_meta