Repository: glotaran/pyglotaran (language: python)

Issue:
[staging] 🐛 Parameters expressions are not evaluated in the correct order
### Version information

- pyglotaran version: staging (v0.8.0-dev)

### Describe the bug

We observed there were differences in the optimization between main (0.7) and staging (0.8-dev) and found the problem to be related to models in which parameters relations (expression) were used. 

After some debugging we found that the order of execution (evaluating parameters) was incorrect, the parameter that had dependencies would itself be added *before* its dependencies causing it to be evaluated with values from a previous iteration. 

### To Reproduce

Steps to reproduce the behavior:

Run any model that has parameters relations in it, for instance:
`study_transient_absorption/transient_absorption_target_analysis.ipynb
on main: [transient_absorption_target_analysis.ipynb`](https://github.com/glotaran/pyglotaran-examples/blob/main/pyglotaran_examples/study_transient_absorption/transient_absorption_target_analysis.ipynb)
on staging: [transient_absorption_target_analysis](https://github.com/glotaran/pyglotaran-examples/blob/staging_rewrite/pyglotaran_examples/study_transient_absorption/transient_absorption_target_analysis.ipynb)

Then observe that the optimization (given sufficient number of iterations) is following a different trajectory.
If no update to the relation parameter happens (e.g. it is fixed, or there are not sufficient iterations/function evaluations possible) you will not see it. Also the first iteration is always idential because the initial parameter values are used.

Important note is that on main the order in which parameters are evlauated is the order in which they were put in the parameters specification file, thus to ensure correctness, relation parameters need to be before the parameters in which they are used in an expresssion. 

### Expected behavior

Both main and staging follow the same optimization trajectory and arrive at the same final result.

### Screenshots

<!-- If applicable, add screenshots to help explain your problem. -->

### Additional context

<!-- Add any other context about the problem here. -->



Discussion / hints:

Fix patch:
diff --git a/glotaran/model/item.py b/glotaran/model/item.py
index 803f994ca..55dbdc7a8 100644
--- a/glotaran/model/item.py
+++ b/glotaran/model/item.py
@@ -314,9 +314,9 @@ def iterate_parameter_fields(
 
 def add_to_initial(label: str, parameters: Parameters, initial: Parameters) -> Parameter:
     if not parameters.has(label):
-        parameters.add(initial.get(label).model_copy())
-        for dep_label in parameters.get(label).get_dependency_parameters():
+        for dep_label in initial.get(label).get_dependency_parameters():
             add_to_initial(dep_label, parameters, initial)
+        parameters.add(initial.get(label).model_copy())
     return parameters.get(label)
 
 


Test patch:
diff --git a/tests/model/test_item.py b/tests/model/test_item.py
index 756e1330a..708a1a237 100644
--- a/tests/model/test_item.py
+++ b/tests/model/test_item.py
@@ -1,10 +1,12 @@
 from __future__ import annotations
 
+from itertools import permutations
 from typing import Literal
 
 from glotaran.model.item import Item
 from glotaran.model.item import ParameterType
 from glotaran.model.item import TypedItem
+from glotaran.model.item import add_to_initial
 from glotaran.model.item import get_item_issues
 from glotaran.model.item import get_structure_and_type_from_field
 from glotaran.model.item import iterate_parameter_fields
@@ -136,3 +138,117 @@ def test_resolve_item_parameters():
     parameters.get("param1").value = 10
     assert parameters.get("param1").value != initial.get("param1").value
     assert resolved.pscalar.value == 10
+
+
+RATES_DICT = {
+    "rates": [
+        ["k1sum", 10, {"vary": False, "non-negative": True}],
+        ["k1", 2.0, {"expr": "$b.1 * $rates.k1sum", "non-negative": True, "vary": False}],
+        ["k2", 8.0, {"expr": "$b.2 * $rates.k1sum", "non-negative": True, "vary": False}],
+        ["k3", 0.54321, {"non-negative": True}],
+    ]
+}
+
+B_DICT = {
+    "b": [
+        ["1", 0.2, {"vary": True, "non-negative": True}],
+        ["2", 0.8, {"expr": "1.0 - $b.1", "non-negative": True, "vary": False}],
+    ]
+}
+
+
+def _add_parameters_to_initial(labels, parameters, initial_parameters):
+    for label in labels:
+        add_to_initial(label, parameters, initial_parameters)
+
+
+def _test_add_to_initial(initial_parameters):
+    # test_ordered_addition
+    parameters = Parameters.empty()
+    _add_parameters_to_initial(
+        ["b.1", "b.2", "rates.k1", "rates.k2"], parameters, initial_parameters
+    )
+    assert list(parameters._parameters.keys()) == [
+        "b.1",
+        "b.2",
+        "rates.k1sum",
+        "rates.k1",
+        "rates.k2",
+    ]
+
+    # test_unordered_addition
+    parameters = Parameters.empty()
+    _add_parameters_to_initial(
+        ["b.2", "b.1", "rates.k1", "rates.k2"], parameters, initial_parameters
+    )
+    assert list(parameters._parameters.keys()) == [
+        "b.1",
+        "b.2",
+        "rates.k1sum",
+        "rates.k1",
+        "rates.k2",
+    ]
+
+    # test_partial_addition
+    parameters = Parameters.empty()
+    _add_parameters_to_initial(["rates.k1", "rates.k2"], parameters, initial_parameters)
+    assert list(parameters._parameters.keys()) == [
+        "b.1",
+        "rates.k1sum",
+        "rates.k1",
+        "b.2",
+        "rates.k2",
+    ]
+
+    # test_all_permutations
+    parameter_labels = ["b.1", "b.2", "rates.k1", "rates.k2", "rates.k3"]
+    for perm in permutations(parameter_labels):
+        parameters = Parameters.empty()
+        for label in perm:
+            add_to_initial(label, parameters, initial_parameters)
+
+        keys_list = list(parameters._parameters.keys())
+
+        assert keys_list.index("rates.k2") > keys_list.index(
+            "b.2"
+        ), f"rates.k2 should come after b.2 for permutation {perm}"
+        assert keys_list.index("b.2") > keys_list.index(
+            "b.1"
+        ), f"b.2 should come after b.1 for permutation {perm}"
+        assert keys_list.index("rates.k1") > keys_list.index(
+            "rates.k1sum"
+        ), f"rates.k1 should come after rates.k1sum for permutation {perm}"
+        assert keys_list.index("rates.k2") > keys_list.index(
+            "rates.k1sum"
+        ), f"rates.k2 should come after rates.k1sum for permutation {perm}"
+
+        assert (
+            parameters.get("b.2").expression == "1.0 - $b.1"
+        ), f"Incorrect expression for b.2. Got: {parameters.get('b.2').expression}"
+        assert (
+            parameters.get("rates.k1").expression == "$b.1 * $rates.k1sum"
+        ), f"Incorrect expression for rates.k1. Got: {parameters.get('rates.k1').expression}"
+        assert (
+            parameters.get("rates.k2").expression == "$b.2 * $rates.k1sum"
+        ), f"Incorrect expression for rates.k2. Got: {parameters.get('rates.k2').expression}"
+
+
+def test_add_to_initial_b_first():
+    """Test the add_to_initial function with 'b' parameters defined first.
+
+    This tests complements `test_add_to_initial_b_last` test, which tests the out of order case.
+    """
+    return _test_add_to_initial(Parameters.from_dict({**B_DICT, **RATES_DICT}))
+
+
+def test_add_to_initial_b_last():
+    """Test the add_to_initial function with 'b' parameters defined last.
+
+    In this way, the 'b' parameters cannot accidentally be 'first'.
+
+    This function tests the behavior of adding parameters to the initial set
+    when the 'b' parameters are defined after the 'rates' parameters in the
+    input dictionary. It ensures that the parameter addition and ordering
+    work correctly regardless of the initial parameter order.
+    """
+    return _test_add_to_initial(Parameters.from_dict({**RATES_DICT, **B_DICT}))