Skip to content

strategies.py's EXPONENTIATED_GRADIENT_MAX_ITER comment describes a value (10) that hasn't been true since it was bumped to 50 #445

Description

@yakew7

Where: faircode/strategies.py:62-70 (EXPONENTIATED_GRADIENT_MAX_ITER's comment vs its actual value).

The gap: the comment directly above the constant argues at length for running at 10 iterations - "even 15 iterations measured at 256s for a single (audit, model) cell - 50 would make a full seven-domain run impractical. 10 iterations still converges well enough to demonstrate the constraint's effect. Raise this for a final, paper-quality run..." - but the constant itself is 50, not 10.

Repro:

$ sed -n '62,70p' faircode/strategies.py
# ExponentiatedGradient deep-copies and refits the base estimator once per
# iteration (its default max_iter is 50). At the row counts several of these
# audits have (Healthcare Readmission ~100k, AI Fair Recruitment ~121k) and
# with GradientBoostingClassifier as the base estimator, even 15 iterations
# measured at 256s for a single (audit, model) cell - 50 would make a full
# seven-domain run impractical. 10 iterations still converges well enough to
# demonstrate the constraint's effect. Raise this for a final,
# paper-quality run where wall-clock time isn't the binding constraint.
EXPONENTIATED_GRADIENT_MAX_ITER = 50

git log -p shows a commit ("Bump EG max_iter to 50 for final paper run") changed only the value, leaving the comment's rationale - built entirely around justifying 10 and warning that 50 is impractical - untouched. fairlearn's own ExponentiatedGradient default is also 50, confirmed via inspect.signature, so this override is currently a silent no-op.

Why it matters: a reader trusting the comment believes S3 (in_processing) runs at a reduced, fast 10 iterations to keep a full seven-domain benchmark run practical - it actually runs fairlearn's full default 50, with the wall-clock cost the comment itself says makes a full run "impractical." Exactly the "docs say X, code does Y" drift this repo tracks as a real bug elsewhere (SPEC.md, MANIFEST_SPEC.md, CITATION.cff).

Suggested fix: either revert to 10 to match the existing comment's documented "fast benchmark" intent, or rewrite the comment to justify 50 as the current deliberate choice and state the actual measured wall-clock cost at 50 iterations across the seven domains.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions