update to be deprecated evaluation_strategy #1682
Conversation
winglian
force-pushed
the
deprecation-fixes
branch
from
4bc55d8
to
61b8ecf
Compare
winglian
force-pushed
the
deprecation-fixes
branch
from
61b8ecf
to
8fda363
Compare
| def validate_evaluation_strategy(cls, evaluation_strategy): | ||
| if evaluation_strategy is not None: | ||
| LOG.warning("evaluation_strategy is deprecated, use eval_strategy instead") | ||
| return evaluation_strategy |
There was a problem hiding this comment.
Do we need to add one validation where we remap eval_strategy = evaluation_strategy if both aren't set. If both are set, we should error out.
| if cfg.fsdp and "bnb" in cfg.optimizer: | ||
| raise ValueError(f"FSDP not compatible with {cfg.optimizer}") |
There was a problem hiding this comment.
This does not exist in our config validation check. Is that okay?
There was a problem hiding this comment.
let's address these in another PR, since these aren't being checked currently
| if cfg.deepspeed and Path(cfg.deepspeed).is_file(): | ||
| with open(cfg.deepspeed, encoding="utf-8") as file: | ||
| contents = file.read() | ||
| deepspeed_cfg: DictDefault = DictDefault(json.loads(contents)) | ||
| if cfg.flash_attention: | ||
| if ( | ||
| deepspeed_cfg.zero_optimization | ||
| and deepspeed_cfg.zero_optimization.stage == 3 | ||
| ): | ||
| if not ( | ||
| ( | ||
| deepspeed_cfg.bf16 | ||
| and deepspeed_cfg.bf16.enabled # pylint: disable=no-member | ||
| is True | ||
| ) | ||
| or ( | ||
| deepspeed_cfg.fp16 | ||
| and deepspeed_cfg.fp16.enabled # pylint: disable=no-member | ||
| is True | ||
| ) | ||
| ): | ||
| raise ValueError( | ||
| "bf16.enabled or fp16.enabled must be set to true when using ZeRO-3 with flash-attention" | ||
| ) | ||
| if "8bit" in cfg.optimizer and deepspeed_cfg.optimizer: | ||
| LOG.warning( | ||
| f"conflicting optimizer: {cfg.optimizer} used alongside deepspeed optimizer." | ||
| ) |
There was a problem hiding this comment.
This does not exist in our config validation check. Is that okay?
There was a problem hiding this comment.
let's address these in another PR, since these aren't being checked currently
|
I see, |
Ah, this is an old enough PR that the c4 issue was pixed in another PR already |
winglian
changed the title
* update to be deprecated evaluation_strategy and c4 dataset * chore: lint * remap eval strategy to new config and add tests
No description provided.