ADD: warning if hub_model_id ist set but not any save strategy #1202
Conversation
JohanWork
changed the title
JohanWork
changed the title
|
@winglian I have update the code and put a test in place, but how do I fix the "Too many public methods (21/20) (too-many-public-methods)" I guess 20 is a default value? I can at least not find where you set it. |
I fixed this for you. thanks! |
| @@ -339,6 +339,11 @@ def validate_config(cfg): | |||
| "push_to_hub_model_id is deprecated. Please use hub_model_id instead." | |||
| ) | |||
|
|
|||
| if cfg.hub_model_id and not (cfg.save_steps or cfg.saves_per_epoch): | |||
There was a problem hiding this comment.
Should this also check for cfg.save_strategy and update the log message.
There was a problem hiding this comment.
Yeah your right @NanoCode012 . The test should check for save_strategy, and the comments should be something like:
hub_model_id is set without any models being saved. To save a model, set save_strategy to steps or leave empty. Dose that sound right? If so I can create a pr and update.
There was a problem hiding this comment.
Yep. Please feel free to tag me for review
There was a problem hiding this comment.
I am a bit confused both save_on_epochs and save_steps needs save_strategy to be steps see: https://github.com/OpenAccess-AI-Collective/axolotl/blob/5a5d47458d9aaf7ead798d15291ba3d9bef785c5/src/axolotl/utils/config.py#L426
https://github.com/OpenAccess-AI-Collective/axolotl/blob/5a5d47458d9aaf7ead798d15291ba3d9bef785c5/src/axolotl/utils/config.py#L442
However https://github.com/OpenAccess-AI-Collective/axolotl/blob/5a5d47458d9aaf7ead798d15291ba3d9bef785c5/README.md?plain=1#L777 says nothing about that, it only says set to no to skip. Should we also maybe update the comment to be somthing like set to 'steps' to save checkpoints?
There was a problem hiding this comment.
save_strategy and save_steps are default HF Trainer Arguments. save_on_epochs is just a handy utility by axolotl that auto computes the save_steps based on total steps, hence needing steps.
This is the doc for save_strategy: https://huggingface.co/docs/transformers/v4.37.2/en/main_classes/trainer#transformers.TrainingArguments.save_strategy
* warning if hub model id set but no save * add warning * move the warning * add test * allow more public methods for tests for now * fix tests --------- Co-authored-by: Wing Lian <[email protected]>
Add warning if hub_model_id is set but not any save strategy. Reference issue