[WIP] Accelerator registry follow-up

[carmocca]

What does this PR do?

My review of #12180

Does your PR introduce any breaking changes? If yes, please list them.

None as this is unreleased.

Before submitting

• Was this discussed/approved 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 internal minor changes/refactorings)

PR review

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

• 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

Did you have fun?

Make sure you had fun coding 🙃

[kaushikb11]

Why have it in accelerator_connector, not during import time __init__?

[carmocca]

Because we don't need to do it on import time, so better to delay it if possible.

[kaushikb11]

Why not? What's the benefit of delaying it?

If I do this

    from pytorch_lightning.accelerators.registry import ACCELERATOR_REGISTRY
    print(ACCELERATOR_REGISTRY.names)

I would be expecting a list of accelerator names, rather than calling one more function _populate_registeries before it. Also registeries is plural, and we are populating only one Registry

[carmocca]

Doing things at import time is usually problematic. In addition to slowing time to start running, it means users are very limited when they want to override or customize behaviour.

Also registeries is plural, and we are populating only one Registry

Yes, but we would eventually do the same changes for strategies.

[kaushikb11]

For strategies, we really can't get rid of register_strategies, as we have multiple configurations for certain strategies.
That's the major con of this approach, we can't have multiple subclasses with different defaults in our code.

We can't have an inconsistent api for registering strategies and accelerators.

[kaushikb11]

The discussion for removing the name method. #12180 (comment)

[carmocca]

I think having name is useful and clearly separates the accelerator from the registry. Otherwise the registry logic (a protected class) leaks inside the Accelerator (a stable class).

It also allows registering the accelerators automatically as shown in this PR.

One drawback I see is that one class couldn't register multiple instances of itself. But we could create subclasses with those defaults.

class MyAccel(Accelerator):
    def __init__(self, a=None):
        ...

    @staticmethod
    @abstractmethod
    def name():
        return "my_accel"



class MyAccelA2(MyAccel):
    def __init__(self, a=2):
        super().__init__(a=a)

    @staticmethod
    @abstractmethod
    def name():
        return "my_accel_a_2"

cc @justusschock @ananthsub as participants from prev discussion