Repository: OpenNMT/OpenNMT-tf
Pull request #976 (2022-09-30 13:09:49)

Do not consider parameter decay_step_duration as a V1 only parameter


Patch:
diff --git a/opennmt/config.py b/opennmt/config.py
index 0e217e50..e4dc7fe1 100644
--- a/opennmt/config.py
+++ b/opennmt/config.py
@@ -324,9 +324,7 @@ def _convert_to_v2_lr_schedules(params):
 
 def _convert_to_v2_step_accumulation(params, config):
     # Try to upgrade step accumulation to train/effective_batch_size.
-    decay_step_duration = _delete_opt(params, "decay_step_duration") or 1
-    gradients_accum = _delete_opt(params, "gradients_accum") or 1
-    accum_steps = max(decay_step_duration, gradients_accum)
+    accum_steps = _delete_opt(params, "gradients_accum") or 1
     if accum_steps > 1:
         train_config = config.setdefault("train", {})
         batch_size = train_config.get("batch_size")
@@ -334,10 +332,18 @@ def _convert_to_v2_step_accumulation(params, config):
             train_config["effective_batch_size"] = batch_size * accum_steps
         else:
             raise ValueError(
-                "params/decay_step_duration and params/gradients_accum "
-                "should be manually converted to train/effective_batch_size"
+                "params/gradients_accum should be manually converted to "
+                "train/effective_batch_size"
             )
 
+        decay_step_duration = params.get("decay_step_duration")
+        if decay_step_duration is not None:
+            decay_step_duration //= accum_steps
+            if decay_step_duration <= 1:
+                _delete_opt(params, "decay_step_duration")
+            else:
+                params["decay_step_duration"] = decay_step_duration
+
 
 def _delete_opt(config, name):
     return config.pop(name, None)


Test patch:
diff --git a/opennmt/tests/config_test.py b/opennmt/tests/config_test.py
index ce88aa73..c5882d7d 100644
--- a/opennmt/tests/config_test.py
+++ b/opennmt/tests/config_test.py
@@ -156,93 +156,111 @@ class ConfigTest(tf.test.TestCase):
             expected_config,
         )
 
-    def testConvertToV2Config(self):
-        v1_config = {
-            "data": {
-                "source_words_vocabulary": "a.txt",
-                "target_words_vocabulary": "b.txt",
-            },
-            "params": {
-                "optimizer": "LazyAdamOptimizer",
-                "optimizer_params": {
-                    "beta1": 0.9,
-                    "beta2": 0.998,
+    @parameterized.expand(
+        [
+            (
+                {
+                    "data": {
+                        "source_words_vocabulary": "a.txt",
+                        "target_words_vocabulary": "b.txt",
+                    },
+                    "params": {
+                        "optimizer": "LazyAdamOptimizer",
+                        "optimizer_params": {
+                            "beta1": 0.9,
+                            "beta2": 0.998,
+                        },
+                        "param_init": 0.1,
+                        "loss_scale": 2,
+                        "horovod": {},
+                        "maximum_learning_rate": 4,
+                        "maximum_iterations": 250,
+                        "clip_gradients": 5,
+                        "weight_decay": 0.1,
+                        "decay_step_duration": 8,
+                        "gradients_accum": 9,
+                        "decay_type": "noam_decay",
+                        "decay_rate": 512,
+                        "decay_steps": 4000,
+                    },
+                    "train": {
+                        "batch_size": 64,
+                        "num_threads": 4,
+                        "prefetch_buffer_size": 1,
+                        "bucket_width": 1,
+                        "train_steps": 500000,
+                        "save_checkpoints_secs": 600,
+                    },
+                    "eval": {
+                        "batch_size": 32,
+                        "eval_delay": 3600,
+                        "exporters": "best",
+                        "num_threads": 4,
+                        "prefetch_buffer_size": 1,
+                    },
+                    "infer": {
+                        "batch_size": 32,
+                        "num_threads": 4,
+                        "prefetch_buffer_size": 1,
+                        "bucket_width": 1,
+                    },
+                    "score": {
+                        "num_threads": 4,
+                        "prefetch_buffer_size": 1,
+                    },
                 },
-                "param_init": 0.1,
-                "loss_scale": 2,
-                "horovod": {},
-                "maximum_learning_rate": 4,
-                "maximum_iterations": 250,
-                "clip_gradients": 5,
-                "weight_decay": 0.1,
-                "decay_step_duration": 8,
-                "gradients_accum": 9,
-                "decay_type": "noam_decay",
-                "decay_rate": 512,
-                "decay_steps": 4000,
-            },
-            "train": {
-                "batch_size": 64,
-                "num_threads": 4,
-                "prefetch_buffer_size": 1,
-                "bucket_width": 1,
-                "train_steps": 500000,
-                "save_checkpoints_secs": 600,
-            },
-            "eval": {
-                "batch_size": 32,
-                "eval_delay": 3600,
-                "exporters": "best",
-                "num_threads": 4,
-                "prefetch_buffer_size": 1,
-            },
-            "infer": {
-                "batch_size": 32,
-                "num_threads": 4,
-                "prefetch_buffer_size": 1,
-                "bucket_width": 1,
-            },
-            "score": {
-                "num_threads": 4,
-                "prefetch_buffer_size": 1,
-            },
-        }
-
-        expected_v2_config = {
-            "data": {
-                "source_vocabulary": "a.txt",
-                "target_vocabulary": "b.txt",
-            },
-            "params": {
-                "optimizer": "LazyAdam",
-                "optimizer_params": {
-                    "beta_1": 0.9,
-                    "beta_2": 0.998,
-                    "clipnorm": 5,
-                    "weight_decay": 0.1,
+                {
+                    "data": {
+                        "source_vocabulary": "a.txt",
+                        "target_vocabulary": "b.txt",
+                    },
+                    "params": {
+                        "optimizer": "LazyAdam",
+                        "optimizer_params": {
+                            "beta_1": 0.9,
+                            "beta_2": 0.998,
+                            "clipnorm": 5,
+                            "weight_decay": 0.1,
+                        },
+                        "decay_type": "NoamDecay",
+                        "decay_params": {
+                            "model_dim": 512,
+                            "warmup_steps": 4000,
+                        },
+                        "maximum_decoding_length": 250,
+                    },
+                    "train": {
+                        "batch_size": 64,
+                        "effective_batch_size": 64 * max(8, 9),
+                        "length_bucket_width": 1,
+                        "max_step": 500000,
+                    },
+                    "eval": {
+                        "batch_size": 32,
+                    },
+                    "infer": {
+                        "batch_size": 32,
+                        "length_bucket_width": 1,
+                    },
                 },
-                "decay_type": "NoamDecay",
-                "decay_params": {
-                    "model_dim": 512,
-                    "warmup_steps": 4000,
+            ),
+            (
+                {"params": {"decay_step_duration": 8}},
+                {"params": {"decay_step_duration": 8}},
+            ),
+            (
+                {
+                    "params": {"decay_step_duration": 8, "gradients_accum": 4},
+                    "train": {"batch_size": 64},
                 },
-                "maximum_decoding_length": 250,
-            },
-            "train": {
-                "batch_size": 64,
-                "effective_batch_size": 64 * max(8, 9),
-                "length_bucket_width": 1,
-                "max_step": 500000,
-            },
-            "eval": {
-                "batch_size": 32,
-            },
-            "infer": {
-                "batch_size": 32,
-                "length_bucket_width": 1,
-            },
-        }
-
+                {
+                    "params": {"decay_step_duration": 2},
+                    "train": {"batch_size": 64, "effective_batch_size": 64 * 4},
+                },
+            ),
+        ]
+    )
+    def testConvertToV2Config(self, v1_config, expected_v2_config):
         original_v1_config = copy.deepcopy(v1_config)
         v2_config = config.convert_to_v2_config(v1_config)
         self.assertDictEqual(v2_config, expected_v2_config)