Repository: DARMA-tasking/LB-analysis-framework

Issue:
Make cluster swap relative threshold a configurable parameter
As suggested by @lifflander in PR #433 , the 5% value in line 117 0f [Execution/lbsClusteringTransferStrategy.py](https://github.com/DARMA-tasking/LB-analysis-framework/pull/433/files/41e8215c721da7c1c729dca393b8affe55f75675#diff-52bdf682017c84ecfdd2f4caa52bf087545beaee905a7211a4ec57b9731a934c) should be made an optional (with default value of 0.05) user-defined parameter

A suggested name is `cluster_swap_rtol`. 

Please make sure that CI is updated accordingly and that this new configuration parameter is tested for invalid types.

Discussion:
cwschilly: @ppebay This PR is now ready for review

Patch:
diff --git a/src/lbaf/Execution/lbsClusteringTransferStrategy.py b/src/lbaf/Execution/lbsClusteringTransferStrategy.py
index fb2f41c..7397eec 100644
--- a/src/lbaf/Execution/lbsClusteringTransferStrategy.py
+++ b/src/lbaf/Execution/lbsClusteringTransferStrategy.py
@@ -22,6 +22,9 @@ class ClusteringTransferStrategy(TransferStrategyBase):
         # Call superclass init
         super(ClusteringTransferStrategy, self).__init__(criterion, parameters, lgr)
 
+        # Initialize cluster swap relative threshold
+        self._cluster_swap_rtol = parameters.get("cluster_swap_rtol",0.05)
+
     def __cluster_objects(self, rank):
         """Cluster migratiable objects by shared block ID when available."""
         # Iterate over all migratable objects on rank
@@ -40,8 +43,9 @@ class ClusteringTransferStrategy(TransferStrategyBase):
             k: clusters[k]
             for k in random.sample(clusters.keys(), len(clusters))}
 
-    def __find_suitable_subclusters(self, clusters, rank_load, r_tol=0.05):
+    def __find_suitable_subclusters(self, clusters, rank_load):
         """Find suitable sub-clusters to bring rank closest and above average load."""
+
         # Bail out early if no clusters are available
         if not clusters:
             self._logger.info("No migratable clusters on rank")
@@ -64,7 +68,7 @@ class ClusteringTransferStrategy(TransferStrategyBase):
                     for p in nr.binomial(n_o, 0.5, n_o))):
                 # Reject subclusters overshooting within relative tolerance
                 reach_load = rank_load - sum([o.get_load() for o in c])
-                if reach_load < (1.0 - r_tol) * self._average_load:
+                if reach_load < (1.0 - self._cluster_swap_rtol) * self._average_load:
                     continue
 
                 # Retain suitable subclusters with their respective distance and cluster
@@ -114,7 +118,8 @@ class ClusteringTransferStrategy(TransferStrategyBase):
                         if c_try > 0.0:
                             # Compute source cluster size only when necessary
                             sz_src = sum([o.get_load() for o in o_src])
-                            if  c_try > 0.05 * sz_src:
+                            self._logger.warning(f"Cluster Tol: {self._cluster_swap_rtol}")
+                            if  c_try > self._cluster_swap_rtol * sz_src:
                                 # Perform swap
                                 self._logger.info(
                                     f"Swapping cluster {k_src} of size {sz_src} with cluster {k_try} on {r_try.get_id()}")
diff --git a/src/lbaf/IO/lbsConfigurationValidator.py b/src/lbaf/IO/lbsConfigurationValidator.py
index 7d8fdae..9f1ee1e 100644
--- a/src/lbaf/IO/lbsConfigurationValidator.py
+++ b/src/lbaf/IO/lbsConfigurationValidator.py
@@ -104,7 +104,7 @@ class ConfigurationValidator:
                 int,
                 lambda x: x > 0,
                 error="Should be of type 'int' and > 0")
-            })
+        })
         self.__from_samplers = Schema({
             "n_ranks": And(
                 int,
@@ -153,6 +153,10 @@ class ConfigurationValidator:
                          str,
                          lambda e: e in ALLOWED_TRANSFER_STRATEGIES,
                          error=f"{get_error_message(ALLOWED_TRANSFER_STRATEGIES)} must be chosen"),
+                         Optional("cluster_swap_rtol"): And(
+                            float,
+                            lambda x: x > 0.0,
+                            error="Should be of type 'float' and magnitude > 0.0"),
                      "criterion": And(
                          str,
                          lambda f: f in ALLOWED_CRITERIA,


Test patch:
diff --git a/tests/unit/config/conf_correct_clustering.yml b/tests/unit/config/conf_correct_clustering.yml
new file mode 100644
index 0000000..615611c
--- /dev/null
+++ b/tests/unit/config/conf_correct_clustering.yml
@@ -0,0 +1,40 @@
+# Specify input
+from_data:
+  data_stem: ../data/synthetic_lb_data/data
+  phase_ids:
+    - 0
+check_schema: False
+
+# Specify work model
+work_model:
+  name: AffineCombination
+  parameters:
+    alpha: 1.0
+    beta: 0.0
+    gamma: 0.0
+
+# Specify algorithm
+algorithm:
+  name: InformAndTransfer
+  phase_id: 0
+  parameters:
+    n_iterations: 4
+    n_rounds: 2
+    fanout: 2
+    order_strategy: arbitrary
+    transfer_strategy: Clustering
+    criterion: Tempered
+    max_objects_per_transfer: 32
+    deterministic_transfer: true
+
+# Specify output
+output_dir: ../output
+output_file_stem: output_file
+LBAF_Viz:
+  x_ranks: 2
+  y_ranks: 2
+  z_ranks: 1
+  object_jitter: 0.5
+  rank_qoi: load
+  object_qoi: load
+  force_continuous_object_qoi: True
diff --git a/tests/unit/config/conf_correct_clustering_set_tol.yml b/tests/unit/config/conf_correct_clustering_set_tol.yml
new file mode 100644
index 0000000..5b8786f
--- /dev/null
+++ b/tests/unit/config/conf_correct_clustering_set_tol.yml
@@ -0,0 +1,41 @@
+# Specify input
+from_data:
+  data_stem: ../data/synthetic_lb_data/data
+  phase_ids:
+    - 0
+check_schema: False
+
+# Specify work model
+work_model:
+  name: AffineCombination
+  parameters:
+    alpha: 1.0
+    beta: 0.0
+    gamma: 0.0
+
+# Specify algorithm
+algorithm:
+  name: InformAndTransfer
+  phase_id: 0
+  parameters:
+    n_iterations: 4
+    n_rounds: 2
+    fanout: 2
+    order_strategy: arbitrary
+    transfer_strategy: Clustering
+    cluster_swap_rtol: 0.07
+    criterion: Tempered
+    max_objects_per_transfer: 32
+    deterministic_transfer: true
+
+# Specify output
+output_dir: ../output
+output_file_stem: output_file
+LBAF_Viz:
+  x_ranks: 2
+  y_ranks: 2
+  z_ranks: 1
+  object_jitter: 0.5
+  rank_qoi: load
+  object_qoi: load
+  force_continuous_object_qoi: True
diff --git a/tests/unit/config/conf_wrong_clustering_set_tol_mag.yml b/tests/unit/config/conf_wrong_clustering_set_tol_mag.yml
new file mode 100644
index 0000000..74c433a
--- /dev/null
+++ b/tests/unit/config/conf_wrong_clustering_set_tol_mag.yml
@@ -0,0 +1,41 @@
+# Specify input
+from_data:
+  data_stem: ../data/synthetic_lb_data/data
+  phase_ids:
+    - 0
+check_schema: False
+
+# Specify work model
+work_model:
+  name: AffineCombination
+  parameters:
+    alpha: 1.0
+    beta: 0.0
+    gamma: 0.0
+
+# Specify algorithm
+algorithm:
+  name: InformAndTransfer
+  phase_id: 0
+  parameters:
+    n_iterations: 4
+    n_rounds: 2
+    fanout: 2
+    order_strategy: arbitrary
+    transfer_strategy: Clustering
+    cluster_swap_rtol: 0.0
+    criterion: Tempered
+    max_objects_per_transfer: 32
+    deterministic_transfer: true
+
+# Specify output
+output_dir: ../output
+output_file_stem: output_file
+LBAF_Viz:
+  x_ranks: 2
+  y_ranks: 2
+  z_ranks: 1
+  object_jitter: 0.5
+  rank_qoi: load
+  object_qoi: load
+  force_continuous_object_qoi: True
diff --git a/tests/unit/config/conf_wrong_clustering_set_tol_type.yml b/tests/unit/config/conf_wrong_clustering_set_tol_type.yml
new file mode 100644
index 0000000..376a8f7
--- /dev/null
+++ b/tests/unit/config/conf_wrong_clustering_set_tol_type.yml
@@ -0,0 +1,41 @@
+# Specify input
+from_data:
+  data_stem: ../data/synthetic_lb_data/data
+  phase_ids:
+    - 0
+check_schema: False
+
+# Specify work model
+work_model:
+  name: AffineCombination
+  parameters:
+    alpha: 1.0
+    beta: 0.0
+    gamma: 0.0
+
+# Specify algorithm
+algorithm:
+  name: InformAndTransfer
+  phase_id: 0
+  parameters:
+    n_iterations: 4
+    n_rounds: 2
+    fanout: 2
+    order_strategy: arbitrary
+    transfer_strategy: Clustering
+    cluster_swap_rtol: 1
+    criterion: Tempered
+    max_objects_per_transfer: 32
+    deterministic_transfer: true
+
+# Specify output
+output_dir: ../output
+output_file_stem: output_file
+LBAF_Viz:
+  x_ranks: 2
+  y_ranks: 2
+  z_ranks: 1
+  object_jitter: 0.5
+  rank_qoi: load
+  object_qoi: load
+  force_continuous_object_qoi: True
diff --git a/tests/unit/test_configuration_validator.py b/tests/unit/test_configuration_validator.py
index 7d2b988..7b8c8be 100644
--- a/tests/unit/test_configuration_validator.py
+++ b/tests/unit/test_configuration_validator.py
@@ -204,6 +204,34 @@ class TestConfig(unittest.TestCase):
             ConfigurationValidator(config_to_validate=configuration, logger=get_logger()).main()
         self.assertEqual(err.exception.args[0], "Should be of type 'list' of 'int' types\nShould be of type 'str' like '0-100'")
 
+    def test_config_validator_correct_clustering(self):
+        with open(os.path.join(self.config_dir, "conf_correct_clustering.yml"), "rt", encoding="utf-8") as config_file:
+            yaml_str = config_file.read()
+            configuration = yaml.safe_load(yaml_str)
+        ConfigurationValidator(config_to_validate=configuration, logger=get_logger()).main()
+
+    def test_config_validator_correct_clustering_set_tol(self):
+        with open(os.path.join(self.config_dir, "conf_correct_clustering_set_tol.yml"), "rt", encoding="utf-8") as config_file:
+            yaml_str = config_file.read()
+            configuration = yaml.safe_load(yaml_str)
+        ConfigurationValidator(config_to_validate=configuration, logger=get_logger()).main()
+
+    def test_config_validator_wrong_clustering_set_tol_type(self):
+        with open(os.path.join(self.config_dir, "conf_wrong_clustering_set_tol_type.yml"), "rt", encoding="utf-8") as config_file:
+            yaml_str = config_file.read()
+            configuration = yaml.safe_load(yaml_str)
+        with self.assertRaises(SchemaError) as err:
+            ConfigurationValidator(config_to_validate=configuration, logger=get_logger()).main()
+        self.assertEqual(err.exception.args[0], "Should be of type 'float' and magnitude > 0.0")
+
+    def test_config_validator_wrong_clustering_set_tol_mag(self):
+        with open(os.path.join(self.config_dir, "conf_wrong_clustering_set_tol_mag.yml"), "rt", encoding="utf-8") as config_file:
+            yaml_str = config_file.read()
+            configuration = yaml.safe_load(yaml_str)
+        with self.assertRaises(SchemaError) as err:
+            ConfigurationValidator(config_to_validate=configuration, logger=get_logger()).main()
+        self.assertEqual(err.exception.args[0], "Should be of type 'float' and magnitude > 0.0")
+
 
 if __name__ == "__main__":
     unittest.main()