Skip to content

Expose negate_key_metric in FedAvg recipes - #4953

Open
chesterxgchen wants to merge 2 commits into
NVIDIA:mainfrom
chesterxgchen:recipe-negate-key-metric
Open

Expose negate_key_metric in FedAvg recipes#4953
chesterxgchen wants to merge 2 commits into
NVIDIA:mainfrom
chesterxgchen:recipe-negate-key-metric

Conversation

@chesterxgchen

Copy link
Copy Markdown
Collaborator

Summary

  • Add negate_key_metric pass-through to the base FedAvg recipe and framework-specific FedAvg recipe wrappers.
  • Wire the flag to IntimeModelSelector so recipe users can select lower-is-better metrics such as losses without reporting a separate negated metric from clients.
  • Add focused recipe tests for PyTorch and NumPy recipe pass-through.

Testing

  • python -m pytest tests/unit_test/recipe/fedavg_recipe_test.py -q
  • git diff --check

@chesterxgchen

Copy link
Copy Markdown
Collaborator Author

/build

@greptile-apps

greptile-apps Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR exposes negate_key_metric through the entire FedAvg recipe stack so users can train on lower-is-better metrics (e.g., losses) without manually negating them on the client side.

  • Base layer: negate_key_metric: bool = False is added to _FedAvgValidator, FedAvgRecipe.__init__, and BaseFedJob.__init__, where it is wired into the auto-created IntimeModelSelector(key_metric=key_metric, negate_key_metric=negate_key_metric).
  • Framework wrappers: All six recipe variants (PyTorch, NumPy, sklearn, KMeans, SVM, TensorFlow) receive and forward the flag consistently to the base recipe.
  • Tests: New parametric tests cover all six framework paths, including a TF stub that bypasses the TensorFlow import requirement at test time.

Confidence Score: 5/5

Safe to merge — the change is a straightforward additive parameter thread-through with no logic changes to existing code paths.

The flag is forwarded correctly at every layer (validator → recipe → BaseFedJob → IntimeModelSelector), IntimeModelSelector already supported the parameter, and all six framework recipe variants are covered by new tests. No existing behavior is altered when negate_key_metric is left at its default value of False.

No files require special attention.

Important Files Changed

Filename Overview
nvflare/recipe/fedavg.py Adds negate_key_metric to _FedAvgValidator, FedAvgRecipe.init, and the BaseFedJob instantiation; correctly stored as self.negate_key_metric and forwarded.
nvflare/job_config/base_fed_job.py Adds negate_key_metric to BaseFedJob and correctly passes it to IntimeModelSelector when auto-created; docstring correctly notes the flag is ignored when a custom model_selector is supplied.
nvflare/app_opt/pt/recipes/fedavg.py Clean pass-through of negate_key_metric to UnifiedFedAvgRecipe with matching docstring update.
nvflare/app_common/np/recipes/fedavg.py Clean pass-through of negate_key_metric to UnifiedFedAvgRecipe with matching docstring update.
nvflare/app_opt/sklearn/recipes/fedavg.py Adds negate_key_metric after key_metric in signature and forwards it to super().init(); consistent with other wrappers.
nvflare/app_opt/sklearn/recipes/kmeans.py Adds negate_key_metric at the end of the constructor signature and forwards it to super().init(); docstring updated.
nvflare/app_opt/sklearn/recipes/svm.py Adds negate_key_metric at the end of the constructor signature and forwards it to super().init(); docstring updated.
nvflare/app_opt/tf/recipes/fedavg.py Adds negate_key_metric after key_metric and forwards it to super().init(); docstring updated correctly.
tests/unit_test/recipe/fedavg_recipe_test.py Adds six new tests covering negate_key_metric pass-through for all framework variants (PT, NumPy, sklearn, KMeans, SVM, TF); TF test uses a careful monkeypatch to avoid requiring TensorFlow at test time.

Sequence Diagram

sequenceDiagram
    participant U as User Code
    participant R as FedAvgRecipe (framework-specific)
    participant BR as FedAvgRecipe (base / unified)
    participant BFJ as BaseFedJob
    participant IMS as IntimeModelSelector

    U->>R: "FedAvgRecipe(key_metric="val_loss", negate_key_metric=True)"
    R->>BR: "super().__init__(key_metric=key_metric, negate_key_metric=negate_key_metric)"
    BR->>BR: _FedAvgValidator stores negate_key_metric
    BR->>BR: "self.negate_key_metric = v.negate_key_metric"
    BR->>BFJ: "BaseFedJob(key_metric=self.key_metric, negate_key_metric=self.negate_key_metric)"
    BFJ->>IMS: "IntimeModelSelector(key_metric="val_loss", negate_key_metric=True)"
    IMS-->>BFJ: selector registered on server
    BFJ-->>BR: job configured
    BR-->>R: recipe ready
    R-->>U: "recipe instance (recipe.negate_key_metric == True)"
Loading

Reviews (4): Last reviewed commit: "Address negate metric recipe review comm..." | Re-trigger Greptile

Comment thread nvflare/job_config/base_fed_job.py
Comment thread tests/unit_test/recipe/fedavg_recipe_test.py
@codecov-commenter

codecov-commenter commented Jul 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 62.85%. Comparing base (d714126) to head (98a994f).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4953      +/-   ##
==========================================
+ Coverage   62.83%   62.85%   +0.02%     
==========================================
  Files         995      995              
  Lines       97868    97870       +2     
==========================================
+ Hits        61496    61520      +24     
+ Misses      36372    36350      -22     
Flag Coverage Δ
unit-tests 62.85% <100.00%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@chesterxgchen

Copy link
Copy Markdown
Collaborator Author

/build

@chesterxgchen
chesterxgchen force-pushed the recipe-negate-key-metric branch from a12889d to 945570a Compare July 23, 2026 18:06
@chesterxgchen

Copy link
Copy Markdown
Collaborator Author

/build

@chesterxgchen
chesterxgchen force-pushed the recipe-negate-key-metric branch from 945570a to 98a994f Compare July 23, 2026 19:06
@chesterxgchen

Copy link
Copy Markdown
Collaborator Author

/build

chesterxgchen added a commit that referenced this pull request Jul 24, 2026
## Summary

- Add `nvflare.client.hf.patch()` for HuggingFace `Trainer` / TRL
`SFTTrainer` federated rounds.
- Support train, evaluate, and submit-model tasks with rank-0 Client API
ownership, DDP coordination, checkpoint-backed Trainer state restore,
budgeted round execution, metrics capture, PEFT adapter scope, and
bf16-to-numpy-safe output conversion.
- Add a clean `examples/advanced/hf_client_api` Qwen SFT/PEFT example
using the default `flare.patch(trainer)` API and a standard
`prepare_data.py` data-prep step, while leaving the legacy
`examples/advanced/llm_hf` example unchanged.
- Add the HF Client API design doc as implemented PR documentation and
keep only one `3rdParty/packaging.LICENSE.txt` file for the optional
`packaging` dependency.
- Add focused fake and real dependency contract tests, including
transformers min/latest and TRL 0.18/latest CI jobs.

## Testing

- `python -m pytest tests/unit_test/app_opt/hf -q`
- `python -m pytest tests/unit_test/recipe/fedavg_recipe_test.py -q`
- `python -m py_compile examples/advanced/hf_client_api/client.py
examples/advanced/hf_client_api/model.py
examples/advanced/hf_client_api/prepare_data.py
examples/advanced/hf_client_api/job.py`
- `python examples/advanced/hf_client_api/client.py --help`
- `python examples/advanced/hf_client_api/prepare_data.py --help`
- `python examples/advanced/hf_client_api/prepare_data.py --n_clients 2
--data_root /private/tmp/nvflare_hf_client_api_qwen_data_prep`
- `python examples/advanced/hf_client_api/job.py --help`
- `python examples/advanced/hf_client_api/job.py --export_config
--job_dir /private/tmp/nvflare_hf_client_api_qwen_job_prepared
--data_root /private/tmp/nvflare_hf_client_api_qwen_data_prep`
- Verified exported launcher config uses `python3 -u custom/client.py
...` rather than an absolute source path under `custom/`
- `git diff --check upstream/main...HEAD`
- `./runtest.sh -s`
- Earlier in this PR: `./runtest.sh -l`

## Review Notes

- The new Qwen example is validated locally through compile/help/export
checks. The README now follows the standard hello-world example layout.
Full simulation requires downloading the Qwen checkpoint and installing
HF/TRL example dependencies.
- FedAvg recipe `negate_key_metric` pass-through was split into separate
PR #4953. This HF PR keeps the Qwen example independent by disabling
recipe best-model selection with `key_metric=""`.
- The TRL contract test constructs real CPU `SFTTrainer` instances and
applies `flare.patch()`, but does not run a real TRL train/send round.
The train/send path is covered by real `transformers.Trainer` and
fake-contract tests.
- `train_with_evaluation` hard-fail policy currently depends on existing
Client API config. The new HF example explicitly calls
`trainer.evaluate()` before `trainer.train()` each round; recipe-level
pass-through can be added later if we want every recipe user to get a
hard failure when pre-train eval is omitted.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants