Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
129 changes: 72 additions & 57 deletions lib/python/examples/fwdllm/DELETION_CANDIDATES.md
Original file line number Diff line number Diff line change
@@ -1,57 +1,72 @@
# fwdllm legacy-code deletion candidates (separate PR, after `launcher-script-fwdllm` merges)

This is the durable tracking doc for what to delete once the launcher
migration PR merges. `PR_CLEANUP_PLAN.md` (the working notes for that PR)
has already been removed from the branch — its still-relevant content is
folded into `../MIGRATING_TO_LAUNCHER.md` (see `MIGRATION_TO_LAUNCHER_FWDLLM.md`
for the current status pointer). This file is the one thing from that
cleanup that survives, so keep it up to date and refer back to it when
opening the deletion PR.

Do NOT delete anything below in the `launcher-script-fwdllm` PR itself — do
it as a follow-up PR once that one is merged and the launcher path has had a
chance to prove itself in real use.

## Confirmed safe to delete

Legacy MPI/JSON launch path, superseded by `flame.launch` + the YAML
experiments in `expt_scripts/`. Nothing in the new path imports any of these.

- `expts/run_tc_expts/json_scripts/` (155 files: 5 legacy aggregator `*.json`
variants + `trainer_0.json..trainer_149.json`). Includes
`aggregator_async_base.json`, `aggregator_async_dynk.json`,
`aggregator_async_maxiter.json` — confirmed no 5th baseline is needed for
these; they're superseded exploratory variants, not preserved
functionality (only `aggregator.json` → `fluxtune_dynkc` and
`aggregator_dynamic_kc.json` → `fluxtune` were carried forward).
- `expts/run_tc_expts/run_three_experiments.sh`
- `expts/run_tc_expts/run_three_parallel.sh`
- `expts/run_tc_expts/run_text_classification.sh`
- `expts/run_tc_expts/launch_single_run.py`
- `expts/run_tc_expts/gpu_mapping.yaml`
- `expts/run_tc_expts/mpi_host_file`
- `expts/run_tc_expts/fedavg_main_tc.py` (not imported by anything else,
unlike its sibling `initializer.py` below)
- `aggregator/fl_main.py`, `trainer/fl_main.py` — confirmed dead via a
repo-wide grep pass: the only references left are inside the
already-doomed `expts/run_tc_expts/` scripts above (`run_three_*.sh`,
`run_text_classification.sh`, `launch_single_run.py`) plus one comment in
`main_fedfwd_agg.py`. Both files use the old argparse+MPI-era
`Config(args.config)` wiring; the live entrypoints
(`aggregator/main_fedfwd_agg.py`, `trainer/main.py`) use
`flame.launch.cli.load_config_from_argv()` instead and already import
every FL class `fl_main.py` used. Same origin commit
(`d29a2f7f`, 2025-01-31), last touched together in `8449fac5`
(2026-05-06) — kept in lockstep by copy/paste until the launcher
migration updated only `main_fedfwd_agg.py`/`main.py`'s `__main__` block.

## Do NOT delete — confirmed still-live dependencies of the new path

- `expts/initializer.py` — imported directly by `aggregator/main_fedfwd_agg.py`
and `trainer/main.py` (the new launcher entrypoints), not just the legacy
`fedavg_main_tc.py`/`fl_main.py`.

## Not code, no action needed

`expts/run_tc_expts/cache_dir/` is already gitignored (253M local data
cache, untracked).
# fwdllm legacy-code deletion candidates (JSON/MPI launch path — separate deletion PR)

**Durable tracking doc for the deletion PR.** `PR_CLEANUP_PLAN.md` (working notes) was removed; its
still-relevant content folded into `../MIGRATING_TO_LAUNCHER.md` (status pointer:
`MIGRATION_TO_LAUNCHER_FWDLLM.md`). This is the survivor — keep it current; it is the manifest for the
deletion PR.

**Precondition NOW MET.** The launcher migration is merged (`#68` launcher-migration readiness via 3
real n=100 runs, `#72` async simulator + fluxtune opts). The live path is `flame.launch` + the YAML
experiments in `expt_scripts/` (`aggregator/main_fedfwd_agg.py`, `trainer/main.py` →
`load_config_from_argv()`), which imports only `expts/initializer.py` from the legacy tree. The
JSON/MPI launch path below is now fully superseded and safe to delete in a **dedicated follow-up PR**
(kept out of the migration PRs so the launcher had time to prove out in real use — it has).

**Why now (operator ask 2026-07-09):** deprecate the JSON-based running. The stale per-trainer /
per-aggregator JSONs risk a **behaviour regression** if someone launches from them by mistake; there
is no reason to keep a second, divergent config path once the launcher works.

Verified via repo-wide grep: nothing on the live path imports any file below; the only references are
inside the doomed files themselves (or commented out). Deletion is safe.
Comment on lines +19 to +20

---

## A. Confirmed safe to delete

### A1. Entire `expts/run_tc_expts/` directory
The whole legacy MPI/JSON launch tree. Contents:
- `json_scripts/` — **155 files**: 5 legacy aggregator variants (`aggregator.json`,
`aggregator_base.json`, `aggregator_async_base.json`, `aggregator_async_dynk.json`,
`aggregator_async_maxiter.json`) + `trainer_0.json … trainer_149.json` (150). Superseded exploratory
variants — only `aggregator.json`→`fluxtune_dynkc` and `aggregator_dynamic_kc.json`→`fluxtune` were
carried forward into `_metadata/baselines.yaml`; no 5th baseline depends on the rest. **These are the
"redundant trainer/aggregator JSONs" the deprecation targets.**
- `fedavg_main_tc.py` — legacy argparse+MPI entrypoint (not imported by the live path).
- `launch_single_run.py`, `run_three_experiments.sh`, `run_three_parallel.sh`,
`run_text_classification.sh` — legacy MPI launch scripts.
- `gpu_mapping.yaml`, `mpi_host_file` — MPI/topology artifacts. `gpu_mapping.yaml` is **vestigial**:
the only readers are `*/fl_main.py` (commented out) and `initializer.py`'s dead argparse builder;
the live trainer hard-sets its device (`FedSgdTrainer.py:391`, spawner pin authoritative — K-D33).
- `cache_dir/` — **already gitignored** (253M local data cache, untracked) → no git action.

### A2. Legacy `fl_main.py` entrypoints
- `aggregator/fl_main.py`, `trainer/fl_main.py` — old argparse+MPI-era `Config(args.config)` wiring,
superseded by `main_fedfwd_agg.py` / `main.py`. Dead via repo-wide grep (only self-references + the
doomed `run_tc_expts/` scripts + one comment in `main_fedfwd_agg.py:96`). Same origin commit
`d29a2f7f`; kept in copy/paste lockstep until the migration updated only the live `__main__` blocks.

## B. Coupled cleanups — do IN THE SAME PR so nothing dangles after deletion

- **`configs/trainer_base.yaml`** — remove the now-dead `gpu_mapping_file: gpu_mapping.yaml` +
`gpu_mapping_key: mapping_myMap` keys (they point at the deleted `run_tc_expts/gpu_mapping.yaml`; no
live reader — device pinning is the spawner's job, K-D33). Leaving them would be a dangling reference.
- **`expt_scripts/diagnose_partition_binning.py`** — `DEF_CACHE` defaults to
`…/run_tc_expts/cache_dir`; repoint to the current cache location (or drop the default). Path-only
default, overridable, points at gitignored data → not a blocker, but tidy it here.
- **`aggregator/main_fedfwd_agg.py:96`** — comment still references the legacy `fl_main.py`; update the
wording (cosmetic).

## C. Do NOT delete — confirmed still-live dependencies of the new path
- `expts/initializer.py` — imported by `aggregator/main_fedfwd_agg.py` (`set_seed, create_model`) and
`trainer/main.py`. Its argparse `--gpu_mapping_*` defaults are dead for the live path (the live
entrypoints call named functions, not the parser) but the module itself is live — **keep**.
- `configs/aggregator_base.json` — **live**: loaded by the current smoke YAMLs
(`expt_scripts/*_n10_smoke.yaml`) and stamped into run snapshots. NOT a legacy JSON — keep.

## D. Post-deletion check
```bash
cd lib/python/examples/fwdllm
grep -rn "run_tc_expts\|fl_main\|fedavg_main_tc\|gpu_mapping" --include=*.py --include=*.sh --include=*.yaml . | grep -v __pycache__
# expect: only expts/initializer.py's (dead) argparse defaults, if the key-cleanup in §B is deferred
bash expt_scripts/run_sequential.sh --run-set main --only fwdllm --mode real --clean --yes # smoke: still launches
```
206 changes: 0 additions & 206 deletions lib/python/examples/fwdllm/aggregator/fl_main.py

This file was deleted.

3 changes: 1 addition & 2 deletions lib/python/examples/fwdllm/aggregator/main_fedfwd_agg.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,7 @@ def post_complete_message(tc_args):
if __name__ == "__main__":
# --log_to_wandb/--wandb_run_name are CLI-only (not in config JSON), mirroring
# async_cifar10/aggregator/pytorch/main_fedavg_agg.py's initialize_wandb()
# gating pattern. No module-level wandb.init() -- gating here is additive
# (the legacy fl_main.py also never called it unconditionally).
# gating pattern. No module-level wandb.init() -- gating here is additive.
_cli_parser = argparse.ArgumentParser(add_help=False)
_cli_parser.add_argument("--log_to_wandb", action="store_true")
_cli_parser.add_argument("--wandb_run_name", type=str, default=None)
Expand Down
8 changes: 4 additions & 4 deletions lib/python/examples/fwdllm/configs/trainer_base.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
# Common properties shared across all trainers; trainer-specific values are
# injected at runtime by flame.launch (see ../../MIGRATING_TO_LAUNCHER.md §9).
#
# Transcribed from expts/run_tc_expts/json_scripts/trainer_1.json and
# aggregator.json (the existing production defaults), not redesigned.
# Originally transcribed from the legacy MPI/JSON launch configs
# (expts/run_tc_expts/, since removed), not redesigned.

backend: mqtt

Expand Down Expand Up @@ -52,8 +52,8 @@ hyperparameters:
# (trainer.dataset.path_style: true skips that lookup entirely).
client_idx: 0
client_num_in_total: 0
gpu_mapping_file: gpu_mapping.yaml
gpu_mapping_key: mapping_myMap
# (gpu_mapping_file/gpu_mapping_key removed with the legacy MPI launch path --
# device pinning is the spawner's job; FedSgdTrainer hard-sets its cuda device.)
data_loader_num_workers: 5

# --- Model ---
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@
# ---- defaults resolved from the run's aggregator_config.json (the /coc/scratch h5s) ----
DEF_PART = "/coc/scratch/dgarg/fl_datasets/fwdllm/fednlp_data/partition_files/agnews_partition.h5"
DEF_DATA = "/coc/scratch/dgarg/fl_datasets/fwdllm/fednlp_data/data_files/agnews_data.h5"
DEF_CACHE = "/home/dgarg39/gaurav/flame/lib/python/examples/fwdllm/expts/run_tc_expts/cache_dir"
# cache lives with the datasets on scratch; override with --cache-dir if elsewhere.
DEF_CACHE = "/coc/scratch/dgarg/fl_datasets/fwdllm/fednlp_data/cache_dir"
DEF_RUN = ("/home/dgarg39/flame/lib/python/examples/fwdllm/experiments/"
"run_20260708_025543_fluxtune_n100_smoke_syn_0_real")
CACHE_TMPL = ("distilbert_distilbert-base-uncased_cached_192_ClassificationModel_"
Expand Down
Loading