Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cyclic foreign vars #177

Closed
wants to merge 9 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions doc/create_model.rst
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,16 @@ In terms of computation and inputs, ``advect_model`` is equivalent to the
``advect_model_raw`` instance created above ; it is just organized
differently.

Avoiding cycles in the model
----------------------------
Often, a process involves updating a variable, that is used by other processes in the model.
This may result in a cycle being detected, and the model not able to run. The
process order is created based on variables with ``intent='out'``. Therefore, any variable
that is created with ``intent='inout'`` will be set last in the calculation order.
Any process that uses this variable as an input, will use the veriable from the previous timestep.
For example, the ``u`` variable in ``ProfileU`` has ``intent='inout'``, and is therefore last in the calculation order.


Update existing models
----------------------

Expand Down
4 changes: 3 additions & 1 deletion doc/framework.rst
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,9 @@ in their computation.
In a model, the processes and their dependencies together form the
nodes and the edges of a Directed Acyclic Graph (DAG). The graph
topology is fully determined by the ``intent`` set for each variable
or foreign variable declared in each process. An ordering that is
or foreign variable declared in each process. That is, a process depends on
another process if and only if the 'parent' process has an ``intent='out'``
variable, that is used by the 'child' process. An ordering that is
computationally consistent can then be obtained using topological
sorting. This is done at Model object creation. The same ordering is
used at every stage of a model run.
Expand Down
43 changes: 43 additions & 0 deletions xsimlab/dot.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
INPUT_EDGE_ATTRS = {"arrowhead": "none", "color": "#b49434"}
VAR_NODE_ATTRS = {"shape": "box", "color": "#555555", "fontcolor": "#555555"}
VAR_EDGE_ATTRS = {"arrowhead": "none", "color": "#555555"}
INOUT_EDGE_ATTRS = {"color": "#000000", "style": "dashed"}


def _hash_variable(var):
Expand Down Expand Up @@ -98,12 +99,46 @@ def _add_var(self, var, p_name):
if var_intent == VarIntent.OUT:
edge_attrs.update({"arrowhead": "empty"})
edge_ends = p_name, var_key
elif var_intent == VarIntent.INOUT:
edge_attrs.update({"arrowhead": "empty"})
edge_ends = p_name, var_key
else:
edge_ends = var_key, p_name

self.g.node(var_key, label=var.name, **node_attrs)
self.g.edge(*edge_ends, weight="200", **edge_attrs)

def add_inout_arrows(self):
for p_name, p_obj in self.model._processes.items():
p_cls = type(p_obj)
for var_name, var in variables_dict(p_cls).items():
# test if the variable is inout
if (
var.metadata["intent"] == VarIntent.INOUT
and var.metadata["var_type"] != VarType.FOREIGN
):
target_keys = _get_target_keys(p_obj, var_name)

# now again cycle through all processes to see if there is a variable with the same reference
for p2_name, p2_obj in self.model._processes.items():
p2_cls = type(p2_obj)

# skip this if it is a dependent process or the process itself
if (
p_name in self.model.dependent_processes[p2_name]
or p_name == p2_name
):
continue

for var2_name, var2 in variables_dict(p2_cls).items():
# if the variable is
target2_keys = _get_target_keys(p2_obj, var2_name)
if len(set(target_keys) & set(target2_keys)):
edge_ends = p_name, p2_name
self.g.edge(
*edge_ends, weight="200", **INOUT_EDGE_ATTRS
)
Comment on lines +111 to +140
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need some work still, would be much easier if we have the deps_dict


def add_inputs(self):
for p_name, var_name in self.model._input_vars:
p_cls = type(self.model[p_name])
Expand Down Expand Up @@ -146,6 +181,7 @@ def to_graphviz(
show_only_variable=None,
show_inputs=False,
show_variables=False,
show_inout_arrows=True,
graph_attr={},
**kwargs,
):
Expand All @@ -167,6 +203,9 @@ def to_graphviz(
elif show_inputs:
builder.add_inputs()

elif show_inout_arrows:
builder.add_inout_arrows()

return builder.get_graph()


Expand Down Expand Up @@ -211,6 +250,7 @@ def dot_graph(
show_only_variable=None,
show_inputs=False,
show_variables=False,
show_inout_arrows=True,
**kwargs,
):
"""
Expand All @@ -236,6 +276,8 @@ def dot_graph(
show_variables : bool, optional
If True, show also the other variables (default: False).
Ignored if `show_only_variable` is not None.
show_inout_arrows : bool, optional
if True, show references to inout variables as dotted lines. (default: True)
**kwargs
Additional keyword arguments to forward to `to_graphviz`.

Expand All @@ -262,6 +304,7 @@ def dot_graph(
show_only_variable=show_only_variable,
show_inputs=show_inputs,
show_variables=show_variables,
show_inout_arrows=show_inout_arrows,
**kwargs,
)

Expand Down
Loading