Repository: uf3/uf3 (language: python)

Issue:
Predictable element and interaction tuple ordering (part 2)
I have implemented a proper sorting of `element_list`.

Previously, there were attempts to sort `element_list`, but it was not being done as intended because the same sorting function is used as the one used to sort each 3-body interaction tuple, which was keeping the first atom fixed.

For instance, if the user provides the `element_list = ['He', 'Li', 'H', 'Be']`, it was keeping the `He` fixed and sorting the other 3. So the sorted `element_list` would have been `['He', 'H', 'Li', 'Be']`.

To sort the entire `element_list` while making minimal changes, I have introduced an optional keyword argument `fix_first` in `sort_interaction_symbols()`. The default behavior is to keep the first element fixed, as we have before to sort the 3-body interaction tuples (so this change is fully backward-compatible by not specifying this keyword argument). Only when the `element_list` is sorted, `fix_first` is set to `False` to allow full sorting without keeping the 1st element fixed.


Discussion / hints:

Fix patch:
diff --git a/uf3/data/composition.py b/uf3/data/composition.py
index 54c49377..7cb975ee 100644
--- a/uf3/data/composition.py
+++ b/uf3/data/composition.py
@@ -49,7 +49,8 @@ def __init__(self,
                 e.g. 2 to fit pair potentials.
         """
         self.degree = degree
-        self.element_list = sort_interaction_symbols(element_list)
+        self.element_list = sort_interaction_symbols(list(set(element_list)),
+                                                     fix_first=False)
         self.numbers = [ase_symbols.symbols2numbers(el).pop()
                         for el in self.element_list]
         self.interactions_map = self.get_interactions_map()
@@ -126,6 +127,7 @@ def get_interactions_map(self) -> Dict[int, Collection[Tuple[str]]]:
                                      key=lambda c: [reference_X[x] for x in c])
         for d in range(3, self.degree + 1):
             combinations = get_element_combinations(self.element_list, d)
+            combinations.sort(key=lambda c: [reference_X[x] for x in c])
             interactions_map[d] = combinations
         return interactions_map
 
@@ -186,12 +188,12 @@ def sort_interaction_map(imap: Dict[Tuple[str], Any]) -> Dict[Tuple[str], Any]:
     return {sort_interaction_symbols(k): v for k, v in imap.items()}
 
 
-def sort_interaction_symbols(symbols: Collection[str]) -> Tuple:
+def sort_interaction_symbols(symbols: Collection[str], fix_first=True) -> Tuple:
     """
     Sort interaction tuple by electronegativities.
     For consistency, many-body interactions (i.e. >2) are sorted while fixing
     the first (center) element."""
-    if len(symbols) >= 3:
+    if len(symbols) >= 3 and fix_first:
         center = symbols[0]
         symbols = sort_elements(symbols[1:])
         symbols.insert(0, center)


Test patch:
diff --git a/tests/test_composition.py b/tests/test_composition.py
index b073c1ea..207df871 100644
--- a/tests/test_composition.py
+++ b/tests/test_composition.py
@@ -23,6 +23,40 @@ def test_ternary(self):
         assert len(handler.interactions_map[2]) == 6
         assert handler.numbers == [13, 29, 40]
 
+    def test_quaternary(self):
+        element_list = ['He', 'Li', 'H', 'Be']  # out of order on purpose
+        handler = ChemicalSystem(element_list, degree=3)
+        assert handler.interactions_map[2] == [
+            ('H', 'H'), ('H', 'He'),  ('H', 'Li'),  ('H', 'Be' ),
+                        ('He', 'He'), ('He', 'Li'), ('He', 'Be'),
+                                      ('Li', 'Li'), ('Li', 'Be'),
+                                                    ('Be', 'Be')
+                                                    ]  # in this order
+        assert handler.interactions_map[3] == [
+            ('H', 'H', 'H'  ), ('H', 'H', 'He'), ('H', 'H', 'Li'), ('H', 'H', 'Be'),
+            ('H', 'He', 'He'), ('H', 'He', 'Li'), ('H', 'He', 'Be'),
+            ('H', 'Li', 'Li'), ('H', 'Li', 'Be'),
+            ('H', 'Be', 'Be'),
+            ('He', 'H', 'H' ), ('He', 'H', 'He'), ('He', 'H', 'Li'), ('He', 'H', 'Be'),
+            ('He', 'He', 'He'), ('He', 'He', 'Li'), ('He', 'He', 'Be'),
+            ('He', 'Li', 'Li'), ('He', 'Li', 'Be'),
+            ('He', 'Be', 'Be'),
+            ('Li', 'H', 'H' ), ('Li', 'H', 'He'), ('Li', 'H', 'Li'), ('Li', 'H', 'Be'),
+            ('Li', 'He', 'He'), ('Li', 'He', 'Li'), ('Li', 'He', 'Be'),
+            ('Li', 'Li', 'Li'), ('Li', 'Li', 'Be'),
+            ('Li', 'Be', 'Be'),
+            ('Be', 'H', 'H' ), ('Be', 'H', 'He'), ('Be', 'H', 'Li'), ('Be', 'H', 'Be'),
+            ('Be', 'He', 'He'), ('Be', 'He', 'Li'), ('Be', 'He', 'Be'),
+            ('Be', 'Li', 'Li'), ('Be', 'Li', 'Be'),
+            ('Be', 'Be', 'Be')
+        ]  # in this order
+        assert handler.numbers == [1, 2, 3, 4,]
+
+    def test_remove_duplicates(self):
+        element_list = ['H', 'H', 'He']
+        handler = ChemicalSystem(element_list)
+        assert handler.numbers == [1, 2]
+
     def test_composition_tuple(self):
         element_list = ['Al', 'Cu', 'Zr']
         handler = ChemicalSystem(element_list)