Skip to content

FDLoRA's implementation, corrected before it ran: what job 183579 would have measured wrong

An independent review of PR #36 found the round-0 average (Algorithm 1, line 7) was computed per source rather than over every client the plan builds, before job 183579 reached its queue slot. The paper's N is every client the server sees; this repository's sources name separate simulated federations for the attack scripts, and the two are not the same population. Fixed by flattening plan into one list before Stage 1 runs and averaging over all of it, so theta_s0 is one value shared across every client regardless of source. Job 183579 was cancelled as invalid before it produced a result; nothing from it is quoted anywhere.

Registered before the next job: when the withheld half is read. -local is saved as of a client's most recent sync, or its Stage 1 value if --sync-period never fired. When the final round is itself a sync round, that value is identical to the transmitted global by construction -- correct and intended only at the deliberate synchronous --sync-period 1 endpoint, where every round syncs and the collision is the point. The default job's first choice, ROUNDS=6 SYNC_PERIOD=3, hit the same construction by coincidence (6 is a multiple of 3), which would have made its withheld/transmitted comparison vacuous without saying so. scripts/fdlora_schedule.py now refuses any rounds/sync_period pair where the last round syncs unless --allow-final-sync states the collision is intended, and the default SYNC_PERIOD moves to 5 -- the paper's own tested value that does not divide 6, so the withheld half is read one round after its last sync rather than the round it was just overwritten in.

Recorded as a limitation rather than resolved by a claim the code does not support. The docstring previously called training each client's global module independently across rounds, rather than simulating the paper's real per-round outer aggregation (Nesterov momentum over every client's change, Algorithm 1 lines 17-18), "the conservative direction... it can only overstate what a real, further-averaged deployment would leak." That claim does not hold: in the real algorithm the server can subtract its previous global from what it receives and recover each client's K-epoch change, T times over, so the paper's own per-client contribution is bounded by roughly inner_steps epochs from a shared seed. This script's saved global module instead carries roughly rounds * inner_steps continuous epochs, uninterrupted by any outer step. A difference in measured leakage against FedDPA may therefore reflect this length gap rather than the schedule, and the comparison is not compute-matched. The docstring now calls it what it is -- an upper bound confounded by training length -- and drops "conservative": overstating leakage is not a safe direction for a measurement that is looking for leakage, it is a bias toward the hypothesis. Implementing the real outer step (rounds outer, clients inner, one Nesterov update a round over every client's change, each round's upload saved) is the fix that removes the confound; it is not done here.

Also fixed, found by the same review: the adapter-shape naming guard in sphragis/experiment/cluster-env.sh classified a selector by testing it as a glob pattern against a synthetic placeholder name (a-c0, a-c0-local, a-c0-p0), which a selector spelled without the assumed hyphen (*local, *-local*) or scoped to an organization prefix (aosp-*) could defeat: *local matched none of the placeholders' suffix-based case arms, so it fell through to the unsuffixed transmitted name while actually selecting only the withheld adapters -- a silent mislabeling where main's original two-shape version had refused the same pattern outright. The guard now globs the real client directories the selector resolves to and classifies each match by its own suffix, so the result depends on what is actually on disk rather than on the selector's spelling.