Skip to content

ADR Suggestion Detecting cyclic dependencies easily with pings #123

@damskii9992

Description

@damskii9992

Discussed in #122

Originally posted by damskii9992 May 16, 2025

General

Since parameters can be made dependent after they're created, it is possible to end up in an infinite loop where a dependent parameters update triggers another dependent parameters update which in turn triggers the first parameters update and so on:

flowchart LR;
   A-->|update|B;
   B-->|update|C;
   C-->|update|D;
   D-->|update|B;
Loading

In ADR #10 a system for detecting cyclic dependencies were suggested and implemented. In this ADR I propose a simpler yet smarter system, which also properly handles the edge-case described in #10.

Current Implementation

In the current implementation, the cyclic dependencies are discovered through the observer pattern with update_ids:
If an update is triggered by a manual change, such as a value change of an independent parameter, the update gets assigned a unique id.
This unique id is then passed to the observers along with the unique_name of the object that triggered the update;

def _notify_observers(self, update_id=None) -> None:
     """Trigger an update in each subscriber."""
     if update_id is None:
        self._global_object.update_id_iterator += 1
        update_id = self._global_object.update_id_iterator
     for observer in self._observers:
         observer._update(update_id=update_id, updating_object=self.unique_name)

In the observers _update method, the update id is checked against an internal dictionary for the updating object. If the stored update id for the updating object is different from the new one, set the stored update id to the new one and proceed with the update, and notify its own observers, passing along the update_id, otherwise raise an error warning that a cyclic dependency has been detected:

def __update(self, update_id: int, updating_object: str) -> None
   if self._dependency_updates[updating_object] == update_id:
      raise RuntimeError
   else:
      update stuff
      self._dependency_updates[updating_object] = update_id
      self._notify_observers(update_id=update_id)

Proposed implementation

In setting up the dependent parameter, in the make_dependent_on method, before any of the parameters values are changed, a ping is sent to all its observers (if any) by calling the _ping_observers method (implemented in the DescriptorNumber):

def _ping_observers(self, ping_origin=None) -> None:
    """Ping all observers to check if any cyclic dependencies has been introduced.

    :param ping_origin: Unique_name of the origin of this ping. Used to avoid cyclic depenencies.
    """
    if ping_origin == self.unique_name:
        raise RuntimeError('\n Cyclic dependency detected!\n' +
                f'An update of {self.unique_name} leads to it updating itself.\n' +
                'Please check your dependencies.')
    if ping_origin is None:
        ping_origin = self.unique_name
    for observer in self._observers:
        observer._ping_observers(ping_origin=ping_origin)

Only in case this dependency-traversal doesn't raise an error, is the dependent parameter updated with the dependency expression.
This approach handles the edge-case described in #10, it simplifies the code base by making the _notify_observers and _update methods simpler, and it get rids of two attributes: the update_id_iterator in the global_object and the _dependency_updates dictionary on Parameter, since the unique_name of the Parameter issuing the ping is now used instead.

Metadata

Metadata

Assignees

No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions