Repository: omni-us/jsonargparse

Issue:
Incorrect instantiation order with nested dependency injection
<!-- If you like this project, please ⭐ star it https://github.com/omni-us/jsonargparse/ -->

## 🐛 Bug report
First of all, this is a really cool project, and I am enjoying it a lot 👍 .
This might be quite niche to be honest, but I encountered unexpected behavior when working on nested classes.
In my use case, the user can choose from multiple loss classes (`Strategy` in the example), which on their own have another nested dependency injection (`NestedStrategy`).
Furthermore, some arguments of the loss are linked to (`StrategyParameter`) and the nested strategy has also arguments provided from somewhere else (`NestedParameter`).

Now in this setting I found that the instantiation order induced by the topological sort of the dependency graph does not enforce that `NestedParameter` is instantiated before `Strategy`.
But when instantiating the `Strategy` object, all actions which target the nested strategy are triggered too.
So in particular the link
```python
parser.link_arguments(
        "nested_param",
        "root.strategy.init_args.nested.init_args.nested_param",
        apply_on="instantiate",
        compute_fn=compute_fn,
    )
```
is triggered however before `NestedStrategy` was instantiated and so the compute_fn is called with the `Namespace` object.

I am not sure whether the graph should contain the dependency or the instantiation logic should not instantiate the actions leading to the nested class?
In `ActionLink.apply_instantiation_links` there is this predicate `is_nested_instantiation_link` (returning False) which hints at the second case being true?

This is how the dependency graph looks like as constructed in `ActionLink.instantiation_order`:
```mermaid
stateDiagram-v2
    nested_param --> root.strategy.init_args.nested,
    param --> root.strategy
```

### To reproduce
I tried to keep the example as small as possible.
We have two dependency injected classes `Strategy` and `NestedStrategy` and two objects which are used for linking `StrategyParameter` and `NestedParameter`.

```python
from jsonargparse import ArgumentParser

class NestedStrategy:

    def __init__(self, nested_param: str):
        self.nested_param = nested_param


class Strategy:

    def __init__(self, param: str, nested: NestedStrategy):
        self.param = param
        self.nested = nested


class Root:

    def __init__(self, strategy: Strategy):
        self.strategy = strategy


class NestedParameter:
    def __init__(self, something_else: str):
        self._something_else = something_else


class StrategyParameter:

    def __init__(self, something: str):
        self.something = something


def compute_fn(nested_param: NestedParameter):
    assert isinstance(
        nested_param, NestedParameter
    ), f"Got wrong type {type(nested_param)}"
    return nested_param


if __name__ == "__main__":
    parser = ArgumentParser()

    parser.add_class_arguments(Root, "root")
    parser.add_class_arguments(NestedParameter, "nested_param")
    parser.add_class_arguments(StrategyParameter, "param")
    parser.link_arguments(
        "nested_param",
        "root.strategy.init_args.nested.init_args.nested_param",
        apply_on="instantiate",
        compute_fn=compute_fn,
    )
    parser.link_arguments(
        "param.param", "root.strategy.init_args.param", apply_on="instantiate"
    )
    parser.add_argument("--config", action="config")
    cfg = parser.parse_args(["--config", "config.yaml"])
    init_cfg = parser.instantiate_classes(cfg)
```

```yaml
root:
  strategy:
    class_path: Strategy
    init_args:
      nested:
        class_path: NestedStrategy
nested_param:
  something_else: "Something Else"

param:
  something: "something"

```

#### Output
```
ValueError: Call to compute_fn of link 'compute_fn(nested_param) --> root.strategy.init_args.nested.init_args.nested_param' with args (Namespace(something_else='Something Else')) failed: Got wrong type. Expected NestedParameter but got <class 'jsonargparse._namespace.Namespace'>
```


### Expected behavior

The `NestedParameter` class is instantiated before passed as parameter to the `compute_fn`.
Note, the `compute_fn` is only to showcase the problem but not necessary to reproduce the bug.

### Environment

- jsonargparse version: jsonargparse 4.34.0
- Python version: Python 3.10.14
- How jsonargparse was installed: pip
- OS: macOS


Discussion:
codecov[bot]: ## [Codecov](https://app.codecov.io/gh/omni-us/jsonargparse/pull/662?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 100.00%. Comparing base [(`2285c1a`)](https://app.codecov.io/gh/omni-us/jsonargparse/commit/2285c1a21eb455f0a702da552e84493778ad4d83?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us) to head [(`8e36057`)](https://app.codecov.io/gh/omni-us/jsonargparse/commit/8e3605742eb38a03ccb09e07304c1a12c1d8dfaf?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us).

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff            @@
##              main      #662   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           22        22           
  Lines         6631      6631           
=========================================
  Hits          6631      6631           
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/omni-us/jsonargparse/pull/662?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=omni-us).


Patch:
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index 38345d1..19c2a29 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -21,6 +21,11 @@ Added
   ``add_argument`` (`#661
   <https://github.com/omni-us/jsonargparse/pull/661>`__).
 
+Fixed
+^^^^^
+- Incorrect instantiation order when instantiation targets share a parent (`#662
+  <https://github.com/omni-us/jsonargparse/pull/662>`__).
+
 
 v4.36.0 (2025-01-17)
 --------------------
diff --git a/jsonargparse/_link_arguments.py b/jsonargparse/_link_arguments.py
index 96b50a0..2c08b0b 100644
--- a/jsonargparse/_link_arguments.py
+++ b/jsonargparse/_link_arguments.py
@@ -18,7 +18,7 @@ from ._actions import (
     _find_parent_action,
     filter_default_actions,
 )
-from ._namespace import Namespace, split_key_leaf
+from ._namespace import Namespace, split_key, split_key_leaf
 from ._parameter_resolvers import get_signature_parameters
 from ._type_checking import ArgumentGroup, ArgumentParser
 
@@ -81,7 +81,10 @@ class DirectedGraph:
         for node in [source, target]:
             if node not in self.nodes:
                 self.nodes.append(node)
-        self.edges_dict[self.nodes.index(source)].append(self.nodes.index(target))
+        source_targets_list = self.edges_dict[self.nodes.index(source)]
+        target_index = self.nodes.index(target)
+        if target_index not in source_targets_list:
+            source_targets_list.append(target_index)
 
     def get_topological_order(self):
         exploring = [False] * len(self.nodes)
@@ -97,7 +100,7 @@ class DirectedGraph:
         for target in self.edges_dict[source]:
             if exploring[target]:
                 raise ValueError(
-                    f"Graph has cycles, found while checking {self.nodes[source]} --> " + self.nodes[target]
+                    f"Graph has cycles, found while checking {self.nodes[source]} --> {self.nodes[target]}"
                 )
             elif not visited[target]:
                 self.topological_sort(target, exploring, visited, order)
@@ -406,11 +409,27 @@ class ActionLink(Action):
     def instantiation_order(parser):
         actions = get_link_actions(parser, "instantiate")
         if actions:
+            targets = set()
             graph = DirectedGraph()
+
+            # Add instantiation links as edges
             for action in actions:
                 target = re.sub(r"\.init_args$", "", split_key_leaf(action.target[0])[0])
                 for _, source_action in action.source:
                     graph.add_edge(source_action.dest, target)
+                targets.add(target)
+
+            # Add instantiation target prefixes as edges
+            targets = sorted(targets, key=lambda x: len(split_key(x)))
+            seen_targets = {targets[0]}
+            for target in targets[1:]:
+                parts = [x.replace("|", ".") for x in target.replace("init_args.", "init_args|").split(".")]
+                for num in range(len(parts) - 1):
+                    target_prefix = ".".join(parts[: num + 1])
+                    if target_prefix in seen_targets:
+                        graph.add_edge(target, target_prefix)
+                seen_targets.add(target)
+
             return graph.get_topological_order()
         return []
 
diff --git a/jsonargparse/_signatures.py b/jsonargparse/_signatures.py
index b16ebdd..48d27e7 100644
--- a/jsonargparse/_signatures.py
+++ b/jsonargparse/_signatures.py
@@ -351,9 +351,11 @@ class SignatureArguments(LoggerProperty):
             annotation = Any
             default = None if is_required else default
             is_required = False
+        is_required_link_target = False
         if is_required and linked_targets is not None and name in linked_targets:
             default = None
             is_required = False
+            is_required_link_target = True
         if (
             kind in {kinds.VAR_POSITIONAL, kinds.VAR_KEYWORD}
             or (not is_required and name[0] == "_")
@@ -371,7 +373,7 @@ class SignatureArguments(LoggerProperty):
             kwargs["help"] = param.doc
         if not is_required:
             kwargs["default"] = default
-            if default is None and not is_optional(annotation, object):
+            if default is None and not is_optional(annotation, object) and not is_required_link_target:
                 annotation = Optional[annotation]
         elif not as_positional:
             kwargs["required"] = True


Test patch:
diff --git a/jsonargparse_tests/test_link_arguments.py b/jsonargparse_tests/test_link_arguments.py
index 2b34154..fbbd170 100644
--- a/jsonargparse_tests/test_link_arguments.py
+++ b/jsonargparse_tests/test_link_arguments.py
@@ -826,6 +826,80 @@ def test_on_instantiate_within_deeper_subclass(parser, caplog):
     assert "Applied link 'encoder.output_channels --> decoder.init_args.input_channels'" in caplog.text
 
 
+class SourceA:
+    def __init__(self, param_a: str):
+        self.attr_a = param_a
+
+
+class SourceB:
+    def __init__(self, param_b: str):
+        self.attr_b = param_b
+
+
+class HierarchyGrandchild:
+    def __init__(self, param_grandchild: str):
+        self.attr_grandchild = param_grandchild
+
+
+class HierarchyChild:
+    def __init__(self, param_child: str, grandchild: HierarchyGrandchild):
+        self.attr_child = param_child
+        self.grandchild = grandchild
+
+
+class HierarchyRoot:
+    def __init__(self, child: HierarchyChild):
+        self.child = child
+
+
+def get_source_a_attr(source_a):
+    assert isinstance(source_a, SourceA)
+    return source_a.attr_a
+
+
+def test_on_instantiate_targets_share_parent(parser):
+    config = {
+        "source_a": {
+            "param_a": "value a",
+        },
+        "source_b": {
+            "param_b": "value b",
+        },
+        "root": {
+            "child": {
+                "class_path": "HierarchyChild",
+                "init_args": {
+                    "grandchild": {
+                        "class_path": "HierarchyGrandchild",
+                    },
+                },
+            },
+        },
+    }
+    parser.add_argument("--config", action="config")
+    parser.add_class_arguments(SourceA, "source_a")
+    parser.add_class_arguments(SourceB, "source_b")
+    parser.add_class_arguments(HierarchyRoot, "root")
+    parser.link_arguments(
+        "source_a",
+        "root.child.init_args.grandchild.init_args.param_grandchild",
+        apply_on="instantiate",
+        compute_fn=get_source_a_attr,
+    )
+    parser.link_arguments("source_b.attr_b", "root.child.init_args.param_child", apply_on="instantiate")
+    cfg = parser.parse_args([f"--config={json.dumps(config)}"])
+    init = parser.instantiate_classes(cfg)
+    assert isinstance(init.source_a, SourceA)
+    assert isinstance(init.source_b, SourceB)
+    assert isinstance(init.root, HierarchyRoot)
+    assert isinstance(init.root.child, HierarchyChild)
+    assert isinstance(init.root.child.grandchild, HierarchyGrandchild)
+    assert init.source_a.attr_a == "value a"
+    assert init.root.child.grandchild.attr_grandchild is init.source_a.attr_a
+    assert init.source_b.attr_b == "value b"
+    assert init.root.child.attr_child is init.source_b.attr_b
+
+
 # link creation failures