[RFC] Simplify accelerator API, add training type argument

[SeanNaren]

🚀 Feature

After the accelerator refactor, we've added some nicety around the TrainingTypePlugin, which now represents all logic that sits on top of an accelerator. However our API seems to be based on the old paradigm where the accelerator/distributed backend were tied, and has bit us already #6089

What I suggest is we do the following:

# Correct
Trainer(gpus=1, accelerator='ddp')  # this is fine. should be set to ddp
Trainer(gpus=1, plugins='ddp_sharded', accelerator='gpu')
Trainer(gpus=1, plugins='ddp_sharded')  # implicit. assume GPU for ddp_sharded as it is the only supported accelerator

# Incorrect
Trainer(gpus=1, accelerator='ddp_sharded')  # throw misconfiguration exception
Trainer(gpus=1, plugins='ddp_sharded', accelerator='ddp') # this should throw a deprecation warning

# The Future

# Plugins can be TrainingTypePlugins, PrecisionPlugins, or whatever the user has provided.
# For precision, we don't do `Trainer(plugins='16bit_precision')` but instead `Trainer(precision=16)` 
# so why do we use the plugins flag for the training type?

Trainer(training_type='ddp', accelerator='gpu')
Trainer(training_type='ddp_spawn', accelerator='cpu')

# so Trainer(plugins=...) is left for cluster_environments, SLURM, ...

Incompatible Plugin/Accelerator

This is something I haven't hashed out but let's say the user does:

Trainer(training_type='ddp_sharded', accelerator='tpu')

This currently is not compatible. We should throw an exception of some sort, but this delves into whitelisting/blacklisting support plugins which could get a bit unwieldy. An alternative is to just assume that the user knows enough about compatibility already.

Backwards compatibility

We allow the TrainingTypePlugin to be specified via the accelerator trainer flag but throw a warning that this will be deprecated in the future.

cc @ananthsub @carmocca @awaelchli @justusschock @tchaton @Borda @kaushikb11 @williamFalcon

[Borda]

just a quick remark, is this a typo?

Trainer(gpus=1, training_type='ddp_spawn', accelerator='cpu')

[carmocca]

These are all the possibilities we currently have

training_types = [
    "single",
    "dp",
    "ddp",
    "ddp2",
    "ddp_spawn",
    "ddp_sharded",
    "ddp_sharded_spawn",
    "horovod",
]
accelerators = ["cpu", "gpu", "tpu"]

import itertools
list(itertools.product(accelerators, training_types))
# comment indicates how it is currently set
[
    ✅ ("cpu", "single"),  # gpus=0
    👎 ("cpu", "ddp"),
    👎 ("cpu", "ddp2"),
    ✅ ("cpu", "ddp_spawn"),  # accelerator=ddp_cpu, num_processes=n
    👎 ("cpu", "ddp_sharded"),
    👎 ("cpu", "ddp_sharded_spawn"),
    👎 ("cpu", "dp"),
    ✅ ("cpu", "horovod"),  # accelerator=horovod

    ✅ ("gpu", "single"),  # gpus=n
    ✅ ("gpu", "ddp"),  # accelerator=ddp, gpus=n
    ✅ ("gpu", "ddp2"),  # accelerator=ddp2, gpus=n
    ✅ ("gpu", "ddp_spawn"),  # accelerator=ddp_spawn, gpus=n
    ✅ ("gpu", "ddp_sharded"),  #  accelerator=ddp, gpus=n, plugins=ddp_sharded
    ✅ ("gpu", "ddp_sharded_spawn"),  #  accelerator=ddp, gpus=n, plugins=ddp_sharded_spawn
    ✅ ("gpu", "dp"),  # accelerator=dp, gpus=n
    ✅ ("gpu", "horovod"),  # accelerator=horovod, gpus=n

    ✅ ("tpu", "single"),  # tpu_cores=n
    👎 ("tpu", "ddp"),
    👎 ("tpu", "ddp2"),
    ✅ ("tpu", "ddp_spawn"),  # accelerator=ddp_spawn, tpu_cores=n
    👎 ("tpu", "ddp_sharded"),
    👎 ("tpu", "ddp_sharded_spawn"),
    👎 ("tpu", "dp"),
    👎 ("tpu", "horovod"),
]

(Might have made a mistake on some, feel free to edit and correct)

DeepSpeed, RPC, RPC sequential are just plugins

We should throw an exception of some sort, but this delves into whitelisting/blacklisting support plugins which could get a bit unwieldy

I don't think we have a choice. I wouldn't leave it to the user to understand how each of these combinations works.
Also no user will be implementing their own training_type or accelerator so it should be safe for us to decide which of these are allowed.

Also, we might want to evaluate standardizing how the number of devices is set. We currently have num_processes, gpus, and tpu_cores. They could be merged into one devices or num_processes

[SeanNaren]

Thanks @carmocca!

Just to play devils advocate, we've gotten this far without having to worry about blacklisting of sorts. Assuming the user knows what they are doing isn't as fatal as we're making it out to believe, as most of these flags requires checking docs to see what is supported. Regardless on the decision of blacklisting/whitelisting I think this should remain a separate endeavour to the API but it is an important extension that the API should support!

I agree on standardising the devices! Definitely a separate case that can extend from this API

[kaushikb11]

IMO, it shouldn't be necessary for the User to pass the accelerator flag and be explicit.

Trainer(training_type='ddp', accelerator='gpu')
Trainer(training_type='ddp_spawn', accelerator='cpu')

Just passing gpus=n or tpu_cores=n should be sufficient, with the CPU accelerator being default.

[carmocca]

IMO, it shouldn't be necessary for the User to pass the accelerator flag and be explicit.

The downside about what you are saying (the current implementation) is that if you want to switch between accelerators, you cannot do it by passing just a different parameter (accelerator='cpu'|'gpu', num_processes=5), instead, you have to use a different argument entirely (num_processes=5 vs gpus=5) which is "harder" programmatically.

This can be confusing for users who don't understand the difference between gpus and num_processes or who might think they need to set both flags. Doing the proposed split would clear this up.

[justusschock]

I also agree with carmocca that having something like num_processes (or maybe num_devices since not all backends use multiple processes) together with a flag that indicates the hardware type is cleaner.
This would be a major API change though

[kaushikb11]

That's one case. How about the cases where users define (accelerator='cpu'|'gpu'|'tpu', gpus=5) or when they just mention gpus=n or tpu_cores=n, should it just be ignored? I think it's a good enough signal to select the accelerator. I feel it's already a bit messy with multiple flags.
Also, for eg., to train it on TPUs, it would be unnecessary to pass both accelerator="tpu" and tpu_cores=n every time, rather than just passing the later one.

[justusschock]

But if you have your trainer for GPUs like this:
Trainer(gpus=8)
and want to switch to TPU, you have to make two changes. One to change the value of gpus and one to change tpu_cores. With the new API, you only had to change accelerator from 'gpu' to 'tpu`

In the deprecation phase, I think we have to use whatever is provided by the current gpus or tpu_cores arguments though.

Another thing this would enable would be something like accelerator='auto', num_processes='auto', where we do smart device detection by ourselves (i.e. we check what's available and use the most advanced thing that's available (TPUs > GPUs > CPUs).

[awaelchli]

In case you are going for this approach I vote for a devices argument which works like the gpus argument today (int, list, string) and remove num_processes instead.

Also I vote for providing good defaults where possible, i.e., I don't see training_type as a mandatory field

[carmocca]

@kaushikb11 can you comment on the status of this issue and what would the next steps be?

[carmocca]

TODOs I could think of:

• Remove DistributedType.DDP_CPU
• Remove DistributedType.TPU_SPAWN
• Rename DistributedType to StrategyType
• Remove distributed_backend references in code and error messages. Remove deprecated trainer flag Trainer.distributed_backend in favor of Trainer.accelerator #9246
• Rename AcceleratorConnector._device_type to _accelerator_type
• Rename AcceleratorConnector._distrib_type to _strategy_type
• Define all non-supported accelerator-strategy combinations. Is this already done?
• Add support for auto. Add support for (accelerator='cpu'|'gpu'|'tpu'|'ipu'|'auto') #7808
• Rename TrainingTypePlugin and its references to StrategyPlugin
• Review existing errors and warnings which still suggest the accelerator=a_strategy API.
• Review and update the docs.

cc @kaushikb11 @tchaton @awaelchli

[carmocca]

Closing this - tracking the previous comment in #9932