Closed stas00 closed 3 years ago
@stas00, thanks for reporting this issue. Can you please set zero_optimization.stage = 2
? This is a requirement for zero_optimization.cpu_offload
?
Thank you, @tjruwase
Would you please add an assert to help the user to know that? Surely silent exit w/o a traceback must be incomplete, right?
When I did that - it moves a long and then crashes:
Traceback (most recent call last):
File "./finetune_trainer.py", line 352, in <module>
main()
File "./finetune_trainer.py", line 289, in main
train_result = trainer.train(
File "/mnt/nvme1/code/huggingface/transformers-deepspeed/src/transformers/trainer.py", line 825, in train
tr_loss += self.training_step(model, inputs)
File "/mnt/nvme1/code/huggingface/transformers-deepspeed/src/transformers/trainer.py", line 1182, in training_step
loss.backward()
File "/home/stas/anaconda3/envs/main-38/lib/python3.8/site-packages/torch/tensor.py", line 233, in backward
torch.autograd.backward(self, gradient, retain_graph, create_graph, inputs=inputs)
File "/home/stas/anaconda3/envs/main-38/lib/python3.8/site-packages/torch/autograd/__init__.py", line 144, in backward
Variable._execution_engine.run_backward(
File "/mnt/nvme1/code/github/00optimize/DeepSpeed/deepspeed/runtime/zero/stage2.py", line 594, in reduce_partition_and_remove_grads
self.reduce_ready_partitions_and_remove_grads(param, i)
File "/mnt/nvme1/code/github/00optimize/DeepSpeed/deepspeed/runtime/zero/stage2.py", line 984, in reduce_ready_partitions_and_remove_grads
self.reduce_independent_p_g_buckets_and_remove_grads(param, i)
File "/mnt/nvme1/code/github/00optimize/DeepSpeed/deepspeed/runtime/zero/stage2.py", line 633, in reduce_independent_p_g_buckets_and_remove_grads
new_grad_tensor = self.ipg_buffer[self.ipg_index].narrow(
AttributeError: 'FP16_DeepSpeedZeroOptimizer' object has no attribute 'ipg_index'
same config as in OP but changed to zero_optimization.stage = 2 + zero_optimization.cpu_offload = true
as you suggested.
I think I see the issue, based on your stack trace.
File "/mnt/nvme1/code/huggingface/transformers-deepspeed/src/transformers/trainer.py", line 1182, in training_step
loss.backward()
Can you please call model.backward()
instead of loss.backward()
? I assume that model is the return value of deepspeed.initialize().
Ah, right, thank you, @tjruwase - I switched to model.module.backward(loss)
- now it wants much more gpu RAM and OOMs even with bs=1. whereas before I could easily do bs=20 or more. And this is using a tiny model which normally takes perhaps 2GB at run time with bs=1. Same config as in OP. If I go back to zero_optimization.stage = 0 + zero_optimization.cpu_offload = false
all is good. memory runs at about 2.5GB max. The lowest capacity card is unfortunately just 8GB.
Any ideas why and what do I need to change to make it work?
Talking about batch size - when does train_batch_size
get used? I noticed that in all my trials until now our trainer was taking care of batching and I didn't really need to use it. However if I set it to "train_batch_size": 1
I get:
Traceback (most recent call last):
File "./finetune_trainer.py", line 352, in <module>
main()
File "./finetune_trainer.py", line 273, in main
trainer = Seq2SeqTrainer(
File "/mnt/nvme1/code/huggingface/transformers-deepspeed/examples/seq2seq/seq2seq_trainer.py", line 57, in __init__
super().__init__(*args, **kwargs)
File "/mnt/nvme1/code/huggingface/transformers-deepspeed/src/transformers/trainer.py", line 249, in __init__
model, optimizer, training_dataloader, lr_scheduler = deepspeed.initialize(
File "/mnt/nvme1/code/github/00optimize/DeepSpeed/deepspeed/__init__.py", line 110, in initialize
engine = DeepSpeedEngine(args=args,
File "/mnt/nvme1/code/github/00optimize/DeepSpeed/deepspeed/runtime/engine.py", line 137, in __init__
self._configure_with_arguments(args, mpu)
File "/mnt/nvme1/code/github/00optimize/DeepSpeed/deepspeed/runtime/engine.py", line 418, in _configure_with_arguments
self._config = DeepSpeedConfig(args.deepspeed_config,
File "/mnt/nvme1/code/github/00optimize/DeepSpeed/deepspeed/runtime/config.py", line 508, in __init__
self._configure_train_batch_size()
File "/mnt/nvme1/code/github/00optimize/DeepSpeed/deepspeed/runtime/config.py", line 636, in _configure_train_batch_size
self._batch_assertion()
File "/mnt/nvme1/code/github/00optimize/DeepSpeed/deepspeed/runtime/config.py", line 575, in _batch_assertion
assert micro_batch > 0, \
AssertionError: Micro batch size per gpu: 0 has to be greater than 0
@stas00 Thanks for the update. It appears that you running into usability gaps, so apologies for the inconvenience, but hopefully this will enable overall improvement for other users.
For zero-offload memory usage, can you please check out this #467 to see if it is relevant?
In the meantime, I will look up an explanation DeepSpeed's handling of train_batch_size
et al.
Please see #131 for a discussion on batch sizes.
For zero-offload memory usage, can you please check out this #467 to see if it is relevant?
This was a perfect reference, thank you!
train_batch_size: Please see #131 for a discussion on batch sizes.
the gist of that thread was:
train_batch_size = #GPUs * train_micro_batch_size_per_gpu * gradient_accumulation_steps
So the setting I was trying was "train_batch_size": 1,
:
and getting:
AssertionError: Micro batch size per gpu: 0 has to be greater than 0
I guess it divided it by 2 gpus and rounded down to 0, right?
It'd probably be a defensive tactic and preventing this kind of questions - for the code to test that the derived train_micro_batch_size_per_gpu
is:
assert train_micro_batch_size_per_gpu > 0, f"train_batch_size has to be at least {n_gpus*gradient_accumulation_steps} (n_gpus*gradient_accumulation_steps), but got {train_batch_size}"
(I haven't looked at the actual variable names, this is just a proof of concept)
but if it was preconfigured it should check it's at least 1.
and another assert that is needed that we discusses elsewhere is:
Going back to train_batch_size
is there a benefit or a requirement to letting DS manage datasets? that is the training_data
arg in deepspeed.initialize
It looks like it'd make things much more complicated for HuggingFace trainer to use that ds feature since we would have to re-init the ds model for each split as the paradigm there is that the model is created once. And the initial experiments appear that letting HF trainer do the batching seems to work just fine.
I'm just not sure what to do with the `"train_batch_size": in the config file then. I am not allowed not to have it there.
What would be your recommendations?
I will put the required asserts on my TODOs, but will appreciate a PR as well if you have the bandwidth.
There is currently no benefit to having deepspeed manage datasets, and our largest examples, bing_bert and megatron-lm, manage their own datasets. So I think your current approach with HF trainer is fine. In future, we might explore innovations in data loading, so watch this space.
Regarding train_batch_size
in the config file when deepspeed is not managing datasets, in that case it used for
(1) detecting gradient accumulation boundaries (because we expect model.step() to be called after every forward/backward), and
(2) computing aggregate throughput.
In reality, train_batch_size
is not required as long as both gradient_accumulation_steps
and train_micro_batch_size_per_gpu
are specified. So my suggestion is that you specify just those two, and things will automatically adjust as you scale up your GPU count, while ensuring that gradient accumulation works correctly.
I will put the required asserts on my TODOs, but will appreciate a PR as well if you have the bandwidth.
The problem is that I'm unfamiliar with those parts of code, it'd be very quick for someone who knows the code to add, and it's not urgent now that you told me what the right mix is.
Thank you for adding those at some point in the future, @tjruwase
There is currently no benefit to having deepspeed manage datasets, and our largest examples, bing_bert and megatron-lm, manage their own datasets. So I think your current approach with HF trainer is fine. In future, we might explore innovations in data loading, so watch this space.
Excellent. Thank you for confirming that!
A related question: what about the optimizer
and lr_scheduler
args of deepspeed.initialize
- my guess is that they might be useful for when the user will want to supply their own if DS doesn't support optimizer
and lr_scheduler
they want. But what about all the special juice that these user-supplied scheduler/optimizer will lack - i.e. do these need to be special to work with ds or would any staple version do?
(Please let me know if it'd better to open a separate issue w/ this question to make it easy to per-use your answers in the future)
Regarding train_batch_size in the config file when deepspeed is not managing datasets, in that case it used for (1) detecting gradient accumulation boundaries (because we expect model.step() to be called after every forward/backward), and (2) computing aggregate throughput.
In reality, train_batch_size is not required as long as both gradient_accumulation_steps and train_micro_batch_size_per_gpu are specified. So my suggestion is that you specify just those two, and things will automatically adjust as you scale up your GPU count, while ensuring that gradient accumulation works correctly.
Awesome! Thank you for the explicit details! That's very helpful!
The problem with your suggestion is this: We tweak all the cl args at the command line (and most ML programs have 100s of those), so remembering to change a config file to match the same cl args is very error-prone and will lead to a lot of mistakes/wasted time. Tweaking BS is most prevalent when dealing with OOM. So having a mismatch would be very error prone.
Could ds reuse some of the args to derive that info from deepspeed.initialize
's args
- which we could adjust if need be to match the desired naming. -i.e. if I pass --batch_size=4
it'd use that as train_batch_size
.
e.g. --fp16
could activate the default fp16
section,--bs
batch size, etc.
Thinking more about it, I think BS is a special case, since half of it is handled by the host program and the other half by DS - so there should be one place where it's set. So if no other cl args from the host program are supported, I strongly believe that this one should be supported.
A few thoughts on cl args vs. config file.
1) We started out passing all deepspeed args on the cl but eventually switched to config file when we realized the lack of standards in naming and semantics of cl args across client codes: e.g., train_batch_size vs batch_size vs effective_batch_size.
2) Besides the difficulty of interpreting cl args, we were also concerned about adding another large number of cl args (now in the dozens and growing) to client scripts, even as we hid the associated parsing logic in deepspeed engine. Also, hierarchical args are a nightmare as cl.
3) After much debate, we resorted to config file to specify parameters that meant only for the deepspeed engine, and not for the client code. There are two obvious downsides to this: (a) duplication of effort by users and (b) inconsistent values for cloned parameters. Some upsides include (a) minimal footprint/impact on client code and (b) easier to add/remove configuration parameters without breaking legacy client code. We accepted these downsides as the lesser (or easier) of two evils, especially compared to trying to interpret the meanings/intentions of cl args across client codes.
4) Some of our users have being bitten by this choice, especially inconsistency between cl and config file clones. But fortunately it is a one-time thing, and once they "get" it, things are smoother. But we are constantly thinking of how to make life easier. Towards that end, we recently support ability to configure deepspeed engine using a dict
arg to deepspeed.initialize() instead of using a config file. With this, the user can convert their cl args into the deepspeed equivalents as appropriate before calling deepspeed.initialize(). I hope this feature can be helpful to you.
Thank you for sharing your detailed notes on the design decisions with regards to the the configuration process.
I do agree that having hundreds of cl args can be difficult to manage. And your config file is a way easier to parse visually.
Here is feedback on using the config file approach so far:
I found that I can't comment things out or make comments so experimenting is somewhat difficult. could there be an alternative format or some variation of straight json that supports comments? This would make the process of tuning up the configuration so much experiment-friendly. Perhaps yml to json intermediate stage could be supported?
Things like 5000000000000000 - are hard to parse too, should support at least 5_000_000_000 - but json doesn't. having 5MB, 5GB shortcuts would be useful too, but definitely not required.
This is diverging from this issue though - please let me know if you'd like to continue this discussion and then I should make a separate issue.
We recently support ability to configure deepspeed engine using a dict arg to deepspeed.initialize() instead of using a config file.
Oh, it's undocumented. Awesome - I'm going to try it - at the moment just passing the batch size would be fantastic. I will keep you posted once I sort it out.
p.s. I just can't say enough how much we appreciate your amazing support - it's absolutely outstanding and very helpful at quickly moving along at integrating your magic engine!
So if I try to pass train_batch_size
via config_params
as you suggested, it fails with:
Traceback (most recent call last):
File "./finetune_trainer.py", line 367, in <module>
Traceback (most recent call last):
File "./finetune_trainer.py", line 367, in <module>
main()
File "./finetune_trainer.py", line 282, in main
main()
File "./finetune_trainer.py", line 282, in main
trainer = Seq2SeqTrainer(
File "/mnt/nvme1/code/huggingface/transformers-deepspeed/src/transformers/trainer.py", line 258, in __init__
trainer = Seq2SeqTrainer(
File "/mnt/nvme1/code/huggingface/transformers-deepspeed/src/transformers/trainer.py", line 258, in __init__
model, optimizer, training_dataloader, lr_scheduler = deepspeed.initialize(
File "/mnt/nvme1/code/github/00optimize/DeepSpeed/deepspeed/__init__.py", line 110, in initialize
engine = DeepSpeedEngine(args=args,
File "/mnt/nvme1/code/github/00optimize/DeepSpeed/deepspeed/runtime/engine.py", line 138, in __init__
model, optimizer, training_dataloader, lr_scheduler = deepspeed.initialize(
File "/mnt/nvme1/code/github/00optimize/DeepSpeed/deepspeed/__init__.py", line 110, in initialize
self._do_sanity_check()
File "/mnt/nvme1/code/github/00optimize/DeepSpeed/deepspeed/runtime/engine.py", line 449, in _do_sanity_check
engine = DeepSpeedEngine(args=args,
File "/mnt/nvme1/code/github/00optimize/DeepSpeed/deepspeed/runtime/engine.py", line 138, in __init__
assert self._is_supported_optimizer(self.optimizer_name()), \
File "/mnt/nvme1/code/github/00optimize/DeepSpeed/deepspeed/runtime/engine.py", line 444, in _is_supported_optimizer
self._do_sanity_check()
File "/mnt/nvme1/code/github/00optimize/DeepSpeed/deepspeed/runtime/engine.py", line 449, in _do_sanity_check
getattr(torch.optim, optimizer_name, None) is not None
TypeError: getattr(): attribute name must be string
assert self._is_supported_optimizer(self.optimizer_name()), \
File "/mnt/nvme1/code/github/00optimize/DeepSpeed/deepspeed/runtime/engine.py", line 444, in _is_supported_optimizer
getattr(torch.optim, optimizer_name, None) is not None
TypeError: getattr(): attribute name must be string
(Grr, I see these are two 2 traces intermixed. Any practical suggestions at how to deal with such situations in general? I guess there should be a way to signal the logger to log only for local_rank=0?)
I did:
ds_config_params = dict(train_batch_size=args.per_device_train_batch_size)
model_parameters = filter(lambda p: p.requires_grad, model.parameters())
model, optimizer, training_dataloader, lr_scheduler = deepspeed.initialize(
args=args,
model=model,
model_parameters=model_parameters,
# optimizer=optimizer,
# lr_scheduler=lr_scheduler,
# training_data=trainset,
config_params=ds_config_params,
)
Does it expect the whole config instead of the config file?
I though I could pass the config file for everything, but train_batch_size
.
As we discussed earlier BS is special when it's needed by both DS and the host application (when DS isn't handling datasets).
And another question:
We have:
args.per_device_eval_batch_size
args.per_device_train_batch_size
which can be quite different. How should we handle this with deepspeed? As I see there is only one batch_size var and its train_batch_size
- no variation for eval.
The evaluation is in general less memory demanding due to not needing gradients, yet it can be quite memory hungry due to beam search and require more memory than training.
Yes, DeepSpeed currently does not support eval/inference execution. The reason is that training was previously the memory and computation intensive part. And so our current examples do evaluation outside of DeepSpeed. But recently, we have seen requests to add support for evaluation/inference. So it is now on our short-term timeline.
Oh, I was completely unaware of this. So I need to do more work then to undo deepspeed post-training stage.
I suppose that I need to remove DeepSpeed layer from DDP(DeepSpeed(OriginalModel)) stack, so it becomes DDP(OriginalModel) as soon as training is done. So something like:
# remove Deepspeed from the stack
self.model_wrapped.module = self.mode_wrapped.module.module
gc.collect() # force memory reclaim immediately
Also need to make sure that there are no memory-holding left-overs from deepspeed, so that all of GPU is available again.
You don't think parts of ZeRO could be of help during eval? I guess there aren't many moving parts as it's then quite localized to each GPU. I'm thinking perhaps ZeRO memory management could save some memory there. But I could be wrong.
@stas00 I typically just retain reference to my original nn.Module
in memory while operating with my model_engine
(also an nn.Module
), and simply perform validation on my validation set with the reference to the original module.
DeepSpeedEngine
only wraps around the original nn.Module
, so validation works just fine with the reference to the original module.
Thank you for your comments, @g-karthik.
Yes, we are recoding things now so that we have:
self.model
- originalself.model_wrapped
- wrapped
so that it's obvious which is which and each is easily available on demand.I suspect we must clear out any references to deepspeed to free the gpu memory if we aren't using it at later stages, but I could be wrong and it already happens automatically. I will discover that once I get a chance to implement it as I didn't realize I needed to remove DS for the evaluation stage, since it worked just fine with it. Perhaps I don't need to do anything at all.
@stas00 I think you'll probably need to free some references for evaluation when the model size is very large, such as perhaps T5-11B. We can discuss this on your transformers
PR, feel free to tag me there so I get an email.
@g-karthik Thanks for sharing your experience with using DeepSpeed for evaluation. @stas00, I will gladly defer to @g-karthik on this topic, as he is quite more knowledgeable than me :).
We are much appreciating you too offering to support our DS integration process, @g-karthik!
@tjruwase, I got a chance to experiment with your suggestions and it is mostly working with some TLC needed for deepspeed.initialize
.
I cannot not pass deepspeed_config
in deepspeed.initialize
's args
- it crashes wanting that arg. Since you said I should be able to send all the config via config_params
, why does it still require deepspeed_config
? And the precedence is unclear then should config_params
provide different from ds_config.js
values.
it requires I pass local_rank
via args too. I guess it's fine. I thought passing all args via config_params
is what was expected but it's no problem. It's just not super clear what goes in args
and what in config_params
. And the first one expects an obj (due to argparse), the second a dict. Not a problem either, a learning curve...
I no longer ask to be able to pass the config file and the overrides, since I have to read the config file in anyway to check that the user hasn't preset anything that we are going to override to avoid errors. So I'm totally fine with just passing a config dict that I sort out myself.
At the moment here is what I came up with:
(Reminding I'm trying to solve the problem of needing to get some config values through HF trainer cl args, but the bulk of it via ds_config.js
)
def _init_deepspeed(self, model):
import io, json
from types import SimpleNamespace
# for clarity extract what args are being passed to deepspeed
# XXX: we shouldn't need to pass deepspeed_config anymore, since we handle it ourselves, but
# currently ds won't work without this argument present in args
ds_args = {k: getattr(self.args, k, None) for k in ["deepspeed_config", "local_rank"]}
with io.open(self.args.deepspeed_config, 'r', encoding='utf-8') as f:
config = json.load(f)
# The following code injects some of trainer's cl args into the DS config
# First to ensure that there is no mismatch between cl args values and presets in the config
# file, ask to not set "train_batch_size", "train_micro_batch_size_per_gpu",
# "gradient_accumulation_steps" in ds config file
bs_keys = ["train_batch_size", "train_micro_batch_size_per_gpu"]
if len([x for x in bs_keys if x in config.keys()]):
raise ValueError(f"Do not include {bs_keys} entries in the ds config file, as they will be set via --per_device_train_batch_size or its default")
if "gradient_accumulation_steps" in config.keys():
raise ValueError(f"Do not include gradient_accumulation_steps entries in the ds config file, as they will be set via --gradient_accumulation_steps or its default")
# DeepSpeed does:
# train_batch_size = n_gpus * train_micro_batch_size_per_gpu * gradient_accumulation_steps
# therefore we just need to set:
config["train_micro_batch_size_per_gpu"] = self.args.per_device_train_batch_size
config["gradient_accumulation_steps"] = self.args.gradient_accumulation_steps
# init that takes some config via `args`, and the bulk of it via `config_params`
model_parameters = filter(lambda p: p.requires_grad, model.parameters())
model, optimizer, _, lr_scheduler = deepspeed.initialize(
args=SimpleNamespace(**ds_args), # expects an obj
model=model,
model_parameters=model_parameters,
config_params=config,
)
return model, optimizer, lr_scheduler
@stas00 Happy New Year. Apologies for the radio silence, I finally succumbed to the holidays.
I want to resume the integration effort starting with this issue of deepspeed.initialize required args.deepspeed_config
. This shouldn't be the case, and so I am investigating now.
Happy New Year to you too, @tjruwase! Yes, it is hard to do co-operative work when those one cooperates with aren't there ;)
I just submitted a fix and unit test. Thanks for catching this bug.
I checked that it is no longer required - thank you for the quick fix.
I'm experimenting with various
zero_optimization
config options and I noticed that when I flip totrue
zero_optimization.cpu_offload
, the application exits w/o crashing or doing any training.leads to a silent exit but doing nothing:
Full log
If I flip
zero_optimization.cpu_offload
tofalse
everything works:Full log
I know I haven't provided reproduction info, as I haven't quite finished working on integration with HF
transformers
, but it should be ready soon. I was hoping you could tell from logs what went wrong. But if it isn't helpful I will update this Issue with reproduction details once I have a transformers branch you could experiment with.