ADD: warning hub model #1301
ADD: warning hub model #1301
Conversation
|
@NanoCode012 happy for you feedback here! |
|
@JohanWork I'm working on a slight refactor of doing validation with Pydantic, so let's fix and merge this after that gets merged. thanks! |
Sure sounds great! When that is merged I can update the pr as well! |
|
@JohanWork the pydantic refactor has been merged. let me know if you have any questions about that. |
src/axolotl/utils/config.py
Outdated
| @@ -358,9 +358,9 @@ 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): | |||
| if cfg.hub_model_id and cfg.save_strategy not in ["steps", "epoch", None]: | |||
There was a problem hiding this comment.
I think this check was made to ensure that one of save_steps/saves_per_epoch/save_strategy was set. In this case, you're missing those conditions?
There was a problem hiding this comment.
Isen't the only thing that matters that you have set a save strategy? https://huggingface.co/docs/transformers/main_classes/trainer#transformers.TrainingArguments.save_strategy. And to not save you have to set it to NO as I have understood it, if you don't set it (None in axolotl) it becomes steps.
There was a problem hiding this comment.
Should this be changed to check for "no" then?
There was a problem hiding this comment.
It could be, but currently the test checks if it is no or any other value. I wanted to check more widely, but can tighten it up if you think it is better @NanoCode012 . Also sorry for slow progress :(
|
Hey @JohanWork , would you be able to rebase or resolve merge conflicts? I will merge this PR after. |
|
@NanoCode012 I have update the pr. Sorry for really slow response from my side. I had to update src/axolotl/utils/config/models/input/v0_4_1/init.py, as previous mention setting save_steps or save_epochs dosen't matter what matters is setting save_stragey(https://huggingface.co/docs/transformers/main_classes/trainer#transformers.TrainingArguments.save_strategy) and None in axolotl defaults to steps. let me know what you think and if I have missed something. Will be faster in responding :) |
|
Thank you for the PR. Sorry that it took a while. |
|
No worries :) Thnx |
Description
Update in accordance to #1202 (comment) . Also fixing and updating the tests etc
Motivation and Context
How has this been tested?
Screenshots (if appropriate)
Types of changes
Social Handles (Optional)