[speediedan]

What does this PR do?

Resolves #16732

Generalize Optimizer validation to accommodate both FSDP 1.x and 2.x

In preparing the downstream package I maintain (finetuning-scheduler) to support PyTorch 2.0's version of FSDP, I noticed that Lightning has yet to be adapted to accommodate FSDP 2.x's useful use_orig_params params mode.

Currently, false-positive "The optimizer does not seem to reference any FSDP parameters" ValueErrors will be triggered when the use_orig_params FSDP configuration is used because such a configuration does not require FlatParameter instances be present in the Optimizer.

https://github.com/Lightning-AI/lightning/blob/c35fee92f4ffdcfe5137c762f614b44ca7f8b7d0/src/lightning/fabric/strategies/fsdp.py#L344-L347

https://github.com/Lightning-AI/lightning/blob/c35fee92f4ffdcfe5137c762f614b44ca7f8b7d0/src/lightning/pytorch/strategies/fsdp.py#L282-L287

FSDP versions 1.13 and 2.0 both ensure FSDP flattened parameters have the _fsdp_flattened attribute set, allowing for a single check to work across both versions (this is the check used internally by PyTorch FSDP). For BC, we need to keep the existing FlatParameter-based check for versions below 1.13, but we should be able to remove that version guard once 1.12 is deprecated.

def _optimizer_has_flat_params(optimizer: Optimizer) -> bool:
    if _TORCH_GREATER_EQUAL_1_13:
        return any(getattr(param, _FSDP_FLATTENED, False) for param in optimizer.param_groups[0]["params"])
    else:
        from torch.distributed.fsdp import FlatParameter

        return any(isinstance(param, FlatParameter) for param in optimizer.param_groups[0]["params"])

I'm sure support for this (along with other FSDP 2.x features) will be arriving as the team's priorities dictate but I'm hoping this PR will allow PyTorch 2.x development/testing to continue for use_orig_params in a BC-compatible manner while also adding some useful 1.x and 2.x FSDP tests.

This PR makes 4 minor changes:

1. Generalizes _optimizer_has_flat_params to accommodate FSDP 1.x and 2.0 usage.
   • To avoid directly depending upon PyTorch's protected _is_fsdp_flattened method and protected constant (which have both changed locations between versions), I've included the relevant constant and attribute check in fabric/strategies/fsdp.py directly rather than relying on a protected attribute whose location is version dependent.
2. Adds an FSDP 2.0 compatible auto_wrap_policy test
3. Adds an FSDP 2.0 _FSDPPolicy-based test that uses use_orig_params to validate the multi-version Optimizer validation while also testing the preferred _FSDPPolicy-based auto-wrapping approach moving forward
4. Guards the 1.x compatible auto_wrap_policy behind the necessary max_torch mark

Tested with: PyTorch 1.12.1, 1.13.1 and 2.0.0.dev20230211

Noted Alternatives

One could use an FSDP instance-based check but that seems both more complicated and brittle. The _fsdp_flattened attribute is used extensively internally and was the best candidate attribute for this check IMHO.

Before submitting

• Was this discussed/agreed via a GitHub issue? (not for typos and docs)
• Did you read the contributor guideline, Pull Request section?
• Did you make sure your PR does only one thing, instead of bundling different changes together?
• Did you make sure to update the documentation with your changes? (if necessary)
• Did you write any new necessary tests? (not for typos and docs)
• Did you verify new and existing tests pass locally with your changes?
• Did you list all the breaking changes introduced by this pull request?
• Did you update the CHANGELOG? (not for typos, docs, test updates, or minor internal changes/refactors)

PR review

Anyone in the community is welcome to review the PR.
Before you start reviewing, make sure you have read the review guidelines. In short, see the following bullet-list:

Reviewer checklist

• Is this pull request ready for review? (if not, please submit in draft mode)
• Check that all items from Before submitting are resolved
• Make sure the title is self-explanatory and the description concisely explains the PR
• Add labels and milestones (and optionally projects) to the PR so it can be classified

[codecov]

Codecov Report

Merging #16733 (f2c998d) into master (0360a6b) will decrease coverage by 9%.
The diff coverage is 0%.

Additional details and impacted files

@@            Coverage Diff            @@
##           master   #16733     +/-   ##
=========================================
- Coverage      69%      59%     -9%     
=========================================
  Files         433      408     -25     
  Lines       31560    31269    -291     
=========================================
- Hits        21625    18556   -3069     
- Misses       9935    12713   +2778     

[awaelchli]

Thanks for the patience on this!

[awaelchli]

🎉