From 978a37a6d4a0baea35c12c13d993f17c1eccc553 Mon Sep 17 00:00:00 2001
From: Johan Hansson <39947546+JohanWork@users.noreply.github.com>
Date: Sun, 18 Feb 2024 20:14:40 +0000
Subject: [PATCH 01/11] update warning for save_strategy

---
 src/axolotl/utils/config.py | 15 +++++++--------
 tests/test_validation.py    |  2 +-
 2 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/src/axolotl/utils/config.py b/src/axolotl/utils/config.py
index 1fc470da9e..3ae8af2654 100644
--- a/src/axolotl/utils/config.py
+++ b/src/axolotl/utils/config.py
@@ -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 != "steps":
         LOG.warning(
-            "hub_model_id is set without any models being saved. To save a model, set either save_steps or saves_per_epoch."
+            "hub_model_id is set without any models being saved. To save a model, set save_strategy to steps or leave empty."
         )
 
     if cfg.gptq and cfg.model_revision:
@@ -423,10 +423,14 @@ def validate_config(cfg):
         raise ValueError(
             "save_steps and saves_per_epoch are mutually exclusive and cannot be used together."
         )
-    if cfg.saves_per_epoch and cfg.save_strategy and cfg.save_strategy != "steps":
+    if cfg.save_strategy and cfg.saves_per_epoch and cfg.save_strategy != "steps":
         raise ValueError(
             "save_strategy must be empty or set to `steps` when used with saves_per_epoch."
         )
+    if cfg.save_strategy and cfg.save_steps and cfg.save_strategy != "steps":
+        raise ValueError(
+            "save_strategy and save_steps mismatch. Please set save_strategy to 'steps' or remove save_steps."
+        )
     if cfg.evals_per_epoch and cfg.eval_steps:
         raise ValueError(
             "eval_steps and evals_per_epoch are mutually exclusive and cannot be used together."
@@ -439,11 +443,6 @@ def validate_config(cfg):
         raise ValueError(
             "evaluation_strategy must be empty or set to `steps` when used with evals_per_epoch."
         )
-    if cfg.save_strategy and cfg.save_steps and cfg.save_strategy != "steps":
-        raise ValueError(
-            "save_strategy and save_steps mismatch. Please set save_strategy to 'steps' or remove save_steps."
-        )
-
     if (
         cfg.evaluation_strategy
         and cfg.eval_steps
diff --git a/tests/test_validation.py b/tests/test_validation.py
index e5a74394cf..b09f66aac6 100644
--- a/tests/test_validation.py
+++ b/tests/test_validation.py
@@ -699,7 +699,7 @@ def test_hub_model_id_save_value_warns(self):
             )
 
     def test_hub_model_id_save_value(self):
-        cfg = DictDefault({"hub_model_id": "test", "saves_per_epoch": 4})
+        cfg = DictDefault({"hub_model_id": "test", "save_strategy": "steps"})
 
         with self._caplog.at_level(logging.WARNING):
             validate_config(cfg)

From ffd8596ec3248b441d3d6b09c6fed8bc52629e2c Mon Sep 17 00:00:00 2001
From: Johan Hansson <39947546+JohanWork@users.noreply.github.com>
Date: Sun, 18 Feb 2024 20:19:52 +0000
Subject: [PATCH 02/11] update

---
 README.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/README.md b/README.md
index 999fd72706..4dd2d55765 100644
--- a/README.md
+++ b/README.md
@@ -774,7 +774,7 @@ lr_quadratic_warmup:
 logging_steps:
 eval_steps: # Leave empty to eval at each epoch, integers for every N steps. decimal for fraction of total steps
 evals_per_epoch: # number of times per epoch to run evals, mutually exclusive with eval_steps
-save_strategy: # Set to `no` to skip checkpoint saves
+save_strategy: # Set to `no` to skip checkpoint saves and steps to save checkpoints
 save_steps: # Leave empty to save at each epoch
 saves_per_epoch: # number of times per epoch to save a checkpoint, mutually exclusive with save_steps
 save_total_limit: # Checkpoints saved at a time

From 19e22961c3feea1375bb4a8449ccd78ffae9f7d8 Mon Sep 17 00:00:00 2001
From: Johan Hansson <39947546+JohanWork@users.noreply.github.com>
Date: Mon, 19 Feb 2024 18:23:10 +0100
Subject: [PATCH 03/11] clean up

---
 README.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/README.md b/README.md
index 4dd2d55765..999fd72706 100644
--- a/README.md
+++ b/README.md
@@ -774,7 +774,7 @@ lr_quadratic_warmup:
 logging_steps:
 eval_steps: # Leave empty to eval at each epoch, integers for every N steps. decimal for fraction of total steps
 evals_per_epoch: # number of times per epoch to run evals, mutually exclusive with eval_steps
-save_strategy: # Set to `no` to skip checkpoint saves and steps to save checkpoints
+save_strategy: # Set to `no` to skip checkpoint saves
 save_steps: # Leave empty to save at each epoch
 saves_per_epoch: # number of times per epoch to save a checkpoint, mutually exclusive with save_steps
 save_total_limit: # Checkpoints saved at a time

From 2f9b8b8ec8b9e50ac62e90f4e48b80e11d418e71 Mon Sep 17 00:00:00 2001
From: Johan Hansson <39947546+JohanWork@users.noreply.github.com>
Date: Mon, 19 Feb 2024 17:55:05 +0000
Subject: [PATCH 04/11] update

---
 src/axolotl/utils/config.py |  4 ++--
 tests/test_validation.py    | 36 +++++++++++++++++++++++++++++++++---
 2 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/src/axolotl/utils/config.py b/src/axolotl/utils/config.py
index 3ae8af2654..7ba0f9f699 100644
--- a/src/axolotl/utils/config.py
+++ b/src/axolotl/utils/config.py
@@ -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 cfg.save_strategy != "steps":
+    if cfg.hub_model_id and cfg.save_strategy not in ["steps", "epoch", None]:
         LOG.warning(
-            "hub_model_id is set without any models being saved. To save a model, set save_strategy to steps or leave empty."
+            "hub_model_id is set without any models being saved. To save a model, set save_strategy to steps, epochs or leave empty."
         )
 
     if cfg.gptq and cfg.model_revision:
diff --git a/tests/test_validation.py b/tests/test_validation.py
index b09f66aac6..f8ab43f185 100644
--- a/tests/test_validation.py
+++ b/tests/test_validation.py
@@ -689,8 +689,17 @@ def test_unfrozen_parameters_w_peft_layers_to_transform(self):
         ):
             validate_config(cfg)
 
-    def test_hub_model_id_save_value_warns(self):
-        cfg = DictDefault({"hub_model_id": "test"})
+    def test_hub_model_id_save_value_warns_save_stragey_no(self):
+        cfg = DictDefault({"hub_model_id": "test", "save_strategy": "no"})
+
+        with self._caplog.at_level(logging.WARNING):
+            validate_config(cfg)
+            assert (
+                "set without any models being saved" in self._caplog.records[0].message
+            )
+
+    def test_hub_model_id_save_value_warns_random_value(self):
+        cfg = DictDefault({"hub_model_id": "test", "save_strategy": "test"})
 
         with self._caplog.at_level(logging.WARNING):
             validate_config(cfg)
@@ -698,13 +707,34 @@ def test_hub_model_id_save_value_warns(self):
                 "set without any models being saved" in self._caplog.records[0].message
             )
 
-    def test_hub_model_id_save_value(self):
+    def test_hub_model_id_save_value_steps(self):
         cfg = DictDefault({"hub_model_id": "test", "save_strategy": "steps"})
 
         with self._caplog.at_level(logging.WARNING):
             validate_config(cfg)
             assert len(self._caplog.records) == 0
 
+    def test_hub_model_id_save_value_epochs(self):
+        cfg = DictDefault({"hub_model_id": "test", "save_strategy": "epoch"})
+
+        with self._caplog.at_level(logging.WARNING):
+            validate_config(cfg)
+            assert len(self._caplog.records) == 0
+
+    def test_hub_model_id_save_value_none(self):
+        cfg = DictDefault({"hub_model_id": "test", "save_strategy": None})
+
+        with self._caplog.at_level(logging.WARNING):
+            validate_config(cfg)
+            assert len(self._caplog.records) == 0
+
+    def test_hub_model_id_save_value_no_set_save_strategy(self):
+        cfg = DictDefault({"hub_model_id": "test"})
+
+        with self._caplog.at_level(logging.WARNING):
+            validate_config(cfg)
+            assert len(self._caplog.records) == 0
+
 
 class ValidationCheckModelConfig(BaseValidation):
     """

From 595ea1dd5b1cf27bc18c3949cfee3393301ba0f4 Mon Sep 17 00:00:00 2001
From: Johan Hansson <39947546+JohanWork@users.noreply.github.com>
Date: Wed, 3 Apr 2024 16:36:42 +0200
Subject: [PATCH 05/11] Update test_validation.py

---
 tests/test_validation.py | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/tests/test_validation.py b/tests/test_validation.py
index a7c8a22bb4..9ab9ed0712 100644
--- a/tests/test_validation.py
+++ b/tests/test_validation.py
@@ -1060,7 +1060,9 @@ def test_hub_model_id_save_value_warns_save_stragey_no(self, minimal_cfg):
             )
 
     def test_hub_model_id_save_value_warns_random_value(self, minimal_cfg):
-        cfg = DictDefault({"hub_model_id": "test", "save_strategy": "test"}) | minimal_cfg
+        cfg = (
+            DictDefault({"hub_model_id": "test", "save_strategy": "test"}) | minimal_cfg
+        )
 
         with self._caplog.at_level(logging.WARNING):
             validate_config(cfg)
@@ -1069,14 +1071,20 @@ def test_hub_model_id_save_value_warns_random_value(self, minimal_cfg):
             )
 
     def test_hub_model_id_save_value_steps(self, minimal_cfg):
-        cfg = DictDefault({"hub_model_id": "test", "save_strategy": "steps"}) | minimal_cfg
+        cfg = (
+            DictDefault({"hub_model_id": "test", "save_strategy": "steps"})
+            | minimal_cfg
+        )
 
         with self._caplog.at_level(logging.WARNING):
             validate_config(cfg)
             assert len(self._caplog.records) == 0
 
     def test_hub_model_id_save_value_epochs(self, minimal_cfg):
-        cfg = DictDefault({"hub_model_id": "test", "save_strategy": "epoch"}) | minimal_cfg
+        cfg = (
+            DictDefault({"hub_model_id": "test", "save_strategy": "epoch"})
+            | minimal_cfg
+        )
 
         with self._caplog.at_level(logging.WARNING):
             validate_config(cfg)

From 800ba08f8f36c710646e8a6d4c6f36bf2c7685f3 Mon Sep 17 00:00:00 2001
From: Johan Hansson <39947546+JohanWork@users.noreply.github.com>
Date: Wed, 17 Apr 2024 08:32:30 +0000
Subject: [PATCH 06/11] fix validation step

---
 src/axolotl/utils/config/models/input/v0_4_1/__init__.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/axolotl/utils/config/models/input/v0_4_1/__init__.py b/src/axolotl/utils/config/models/input/v0_4_1/__init__.py
index 6eaf34c547..2264c38cd6 100644
--- a/src/axolotl/utils/config/models/input/v0_4_1/__init__.py
+++ b/src/axolotl/utils/config/models/input/v0_4_1/__init__.py
@@ -689,10 +689,10 @@ def check_saves(cls, data):
     @classmethod
     def check_push_save(cls, data):
         if data.get("hub_model_id") and not (
-            data.get("save_steps") or data.get("saves_per_epoch")
+            data.get("save_strategy")
         ):
             LOG.warning(
-                "hub_model_id is set without any models being saved. To save a model, set either save_steps or saves_per_epoch."
+                "hub_model_id is set without any models being saved. To save a model, set save_strategy."
             )
         return data
 

From b57dd7c2eb85c4b3f8b49f222480b8f9ef6f6645 Mon Sep 17 00:00:00 2001
From: Johan Hansson <39947546+JohanWork@users.noreply.github.com>
Date: Wed, 17 Apr 2024 08:45:17 +0000
Subject: [PATCH 07/11] update

---
 .../utils/config/models/input/v0_4_1/__init__.py     |  4 +---
 tests/test_validation.py                             | 12 ++++++++----
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/src/axolotl/utils/config/models/input/v0_4_1/__init__.py b/src/axolotl/utils/config/models/input/v0_4_1/__init__.py
index 2264c38cd6..33282821bf 100644
--- a/src/axolotl/utils/config/models/input/v0_4_1/__init__.py
+++ b/src/axolotl/utils/config/models/input/v0_4_1/__init__.py
@@ -688,9 +688,7 @@ def check_saves(cls, data):
     @model_validator(mode="before")
     @classmethod
     def check_push_save(cls, data):
-        if data.get("hub_model_id") and not (
-            data.get("save_strategy")
-        ):
+        if data.get("hub_model_id") and not (data.get("save_strategy")):
             LOG.warning(
                 "hub_model_id is set without any models being saved. To save a model, set save_strategy."
             )
diff --git a/tests/test_validation.py b/tests/test_validation.py
index 9ab9ed0712..ac5412f2ef 100644
--- a/tests/test_validation.py
+++ b/tests/test_validation.py
@@ -1093,16 +1093,20 @@ def test_hub_model_id_save_value_epochs(self, minimal_cfg):
     def test_hub_model_id_save_value_none(self, minimal_cfg):
         cfg = DictDefault({"hub_model_id": "test", "save_strategy": None}) | minimal_cfg
 
-        with self._caplog.at_level(logging.WARNING):
+        with pytest.raises(
+            ValueError,
+            match=r".*hub_model_id is set without any models being saved. To save a model, set save_strategy*",
+        ):
             validate_config(cfg)
-            assert len(self._caplog.records) == 0
 
     def test_hub_model_id_save_value_no_set_save_strategy(self, minimal_cfg):
         cfg = DictDefault({"hub_model_id": "test"}) | minimal_cfg
 
-        with self._caplog.at_level(logging.WARNING):
+        with pytest.raises(
+            ValueError,
+            match=r".*hub_model_id is set without any models being saved. To save a model, set save_strategy*",
+        ):
             validate_config(cfg)
-            assert len(self._caplog.records) == 0
 
 
 class TestValidationCheckModelConfig(BaseValidation):

From f0d319c6da91eb02fa6bddaae071c27e958bd574 Mon Sep 17 00:00:00 2001
From: Johan Hansson <39947546+JohanWork@users.noreply.github.com>
Date: Wed, 17 Apr 2024 08:57:06 +0000
Subject: [PATCH 08/11] test_validation

---
 .../utils/config/models/input/v0_4_1/__init__.py   |  4 +++-
 tests/test_validation.py                           | 14 +++++++-------
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/src/axolotl/utils/config/models/input/v0_4_1/__init__.py b/src/axolotl/utils/config/models/input/v0_4_1/__init__.py
index 33282821bf..7e91a70269 100644
--- a/src/axolotl/utils/config/models/input/v0_4_1/__init__.py
+++ b/src/axolotl/utils/config/models/input/v0_4_1/__init__.py
@@ -688,7 +688,9 @@ def check_saves(cls, data):
     @model_validator(mode="before")
     @classmethod
     def check_push_save(cls, data):
-        if data.get("hub_model_id") and not (data.get("save_strategy")):
+        if data.get("hub_model_id") and (
+            data.get("save_strategy") not in ["steps", "epoch"]
+        ):
             LOG.warning(
                 "hub_model_id is set without any models being saved. To save a model, set save_strategy."
             )
diff --git a/tests/test_validation.py b/tests/test_validation.py
index ac5412f2ef..e295402c77 100644
--- a/tests/test_validation.py
+++ b/tests/test_validation.py
@@ -1053,11 +1053,11 @@ def test_unfrozen_parameters_w_peft_layers_to_transform(self, minimal_cfg):
     def test_hub_model_id_save_value_warns_save_stragey_no(self, minimal_cfg):
         cfg = DictDefault({"hub_model_id": "test", "save_strategy": "no"}) | minimal_cfg
 
-        with self._caplog.at_level(logging.WARNING):
+        with pytest.raises(
+            ValueError,
+            match=r".*hub_model_id is set without any models being saved*",
+        ):
             validate_config(cfg)
-            assert (
-                "set without any models being saved" in self._caplog.records[0].message
-            )
 
     def test_hub_model_id_save_value_warns_random_value(self, minimal_cfg):
         cfg = (
@@ -1067,7 +1067,7 @@ def test_hub_model_id_save_value_warns_random_value(self, minimal_cfg):
         with self._caplog.at_level(logging.WARNING):
             validate_config(cfg)
             assert (
-                "set without any models being saved" in self._caplog.records[0].message
+            match=r".*hub_model_id is set without any models being saved*",
             )
 
     def test_hub_model_id_save_value_steps(self, minimal_cfg):
@@ -1095,7 +1095,7 @@ def test_hub_model_id_save_value_none(self, minimal_cfg):
 
         with pytest.raises(
             ValueError,
-            match=r".*hub_model_id is set without any models being saved. To save a model, set save_strategy*",
+            match=r".*hub_model_id is set without any models being saved*",
         ):
             validate_config(cfg)
 
@@ -1104,7 +1104,7 @@ def test_hub_model_id_save_value_no_set_save_strategy(self, minimal_cfg):
 
         with pytest.raises(
             ValueError,
-            match=r".*hub_model_id is set without any models being saved. To save a model, set save_strategy*",
+            match=r".*hub_model_id is set without any models being saved*",
         ):
             validate_config(cfg)
 

From 0570ab8cc0083fda90ec930ba411881f01329cf7 Mon Sep 17 00:00:00 2001
From: Johan Hansson <39947546+JohanWork@users.noreply.github.com>
Date: Wed, 17 Apr 2024 09:00:14 +0000
Subject: [PATCH 09/11] update

---
 tests/test_validation.py | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/test_validation.py b/tests/test_validation.py
index e295402c77..eaf9473775 100644
--- a/tests/test_validation.py
+++ b/tests/test_validation.py
@@ -1064,11 +1064,11 @@ def test_hub_model_id_save_value_warns_random_value(self, minimal_cfg):
             DictDefault({"hub_model_id": "test", "save_strategy": "test"}) | minimal_cfg
         )
 
-        with self._caplog.at_level(logging.WARNING):
+        with pytest.raises(
+            ValueError,
+            match=r".*can have unexpected behavior*",
+        ):
             validate_config(cfg)
-            assert (
-            match=r".*hub_model_id is set without any models being saved*",
-            )
 
     def test_hub_model_id_save_value_steps(self, minimal_cfg):
         cfg = (

From 6f922d332e036fd1857ada653ee511a56b585e83 Mon Sep 17 00:00:00 2001
From: Johan Hansson <39947546+JohanWork@users.noreply.github.com>
Date: Wed, 17 Apr 2024 09:18:31 +0000
Subject: [PATCH 10/11] fix

---
 .../config/models/input/v0_4_1/__init__.py    |  2 +-
 tests/test_validation.py                      | 24 +++++++------------
 2 files changed, 9 insertions(+), 17 deletions(-)

diff --git a/src/axolotl/utils/config/models/input/v0_4_1/__init__.py b/src/axolotl/utils/config/models/input/v0_4_1/__init__.py
index 7e91a70269..b64d5448e2 100644
--- a/src/axolotl/utils/config/models/input/v0_4_1/__init__.py
+++ b/src/axolotl/utils/config/models/input/v0_4_1/__init__.py
@@ -689,7 +689,7 @@ def check_saves(cls, data):
     @classmethod
     def check_push_save(cls, data):
         if data.get("hub_model_id") and (
-            data.get("save_strategy") not in ["steps", "epoch"]
+            data.get("save_strategy") not in ["steps", "epoch", None]
         ):
             LOG.warning(
                 "hub_model_id is set without any models being saved. To save a model, set save_strategy."
diff --git a/tests/test_validation.py b/tests/test_validation.py
index eaf9473775..b7b8a8c885 100644
--- a/tests/test_validation.py
+++ b/tests/test_validation.py
@@ -1053,22 +1053,18 @@ def test_unfrozen_parameters_w_peft_layers_to_transform(self, minimal_cfg):
     def test_hub_model_id_save_value_warns_save_stragey_no(self, minimal_cfg):
         cfg = DictDefault({"hub_model_id": "test", "save_strategy": "no"}) | minimal_cfg
 
-        with pytest.raises(
-            ValueError,
-            match=r".*hub_model_id is set without any models being saved*",
-        ):
+        with self._caplog.at_level(logging.WARNING):
             validate_config(cfg)
+            assert len(self._caplog.records) == 1
 
     def test_hub_model_id_save_value_warns_random_value(self, minimal_cfg):
         cfg = (
             DictDefault({"hub_model_id": "test", "save_strategy": "test"}) | minimal_cfg
         )
 
-        with pytest.raises(
-            ValueError,
-            match=r".*can have unexpected behavior*",
-        ):
+        with self._caplog.at_level(logging.WARNING):
             validate_config(cfg)
+            assert len(self._caplog.records) == 1
 
     def test_hub_model_id_save_value_steps(self, minimal_cfg):
         cfg = (
@@ -1093,20 +1089,16 @@ def test_hub_model_id_save_value_epochs(self, minimal_cfg):
     def test_hub_model_id_save_value_none(self, minimal_cfg):
         cfg = DictDefault({"hub_model_id": "test", "save_strategy": None}) | minimal_cfg
 
-        with pytest.raises(
-            ValueError,
-            match=r".*hub_model_id is set without any models being saved*",
-        ):
+        with self._caplog.at_level(logging.WARNING):
             validate_config(cfg)
+            assert len(self._caplog.records) == 0
 
     def test_hub_model_id_save_value_no_set_save_strategy(self, minimal_cfg):
         cfg = DictDefault({"hub_model_id": "test"}) | minimal_cfg
 
-        with pytest.raises(
-            ValueError,
-            match=r".*hub_model_id is set without any models being saved*",
-        ):
+        with self._caplog.at_level(logging.WARNING):
             validate_config(cfg)
+            assert len(self._caplog.records) == 1
 
 
 class TestValidationCheckModelConfig(BaseValidation):

From 6d5efc66e2c30945cd969bcd97a9e67e83b6611f Mon Sep 17 00:00:00 2001
From: Johan Hansson <39947546+JohanWork@users.noreply.github.com>
Date: Wed, 17 Apr 2024 09:24:47 +0000
Subject: [PATCH 11/11] fix

---
 tests/test_validation.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/test_validation.py b/tests/test_validation.py
index b7b8a8c885..c6a8455eab 100644
--- a/tests/test_validation.py
+++ b/tests/test_validation.py
@@ -1098,7 +1098,7 @@ def test_hub_model_id_save_value_no_set_save_strategy(self, minimal_cfg):
 
         with self._caplog.at_level(logging.WARNING):
             validate_config(cfg)
-            assert len(self._caplog.records) == 1
+            assert len(self._caplog.records) == 0
 
 
 class TestValidationCheckModelConfig(BaseValidation):