Skip to content

Commit

Permalink
• composition.py:
Browse files Browse the repository at this point in the history 
  - _determine_node_roles:
     fix bug in which nested comp was prevented from being an OUTPUT Node if,
     in addition to Nodes that qualifed as OUTPUT, it also had nodes that projected
     to Nodes in an outer comp (making it look like it was INTERNAL)
  • Loading branch information
jdcpni committed Dec 12, 2021
1 parent 6fa1cbc commit 90b134b
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 58 deletions.
52 changes: 12 additions & 40 deletions psyneulink/core/compositions/composition.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4383,44 +4383,8 @@ def _determine_node_roles(self, context=None):
NodeRole.FEEDBACK_RECEIVER
)

# region
# # MODIFIED 4/25/20 OLD NOTES:
# # If no OUTPUT nodes were explicitly specified as required_roles by *user* , assign them:
# # - if there are LearningMechanisms, OUTPUT node is the last non-learning-related node.
# # - if there are no TERMINAL nodes either, then the last node added to the Composition becomes the OUTPUT node.
# # - ignore OUTPUT nodes in learning pathways as those are assigned automatically in add_linear_learning_pathway
# # and don't want that to suppress normal assignment of TERMINAL nodes in non-learning pathways as OUTPUT nodes
# # (if user has not specified any as required roles)
# # # Assign TERMINAL role to nodes that are last in the scheduler's consideration queue that are:
# # # - not used for Learning;
# # # - not ControlMechanisms or ObjectiveMechanisms that project to them;
# # # - do not project to any other nodes.
# #
# # # First, find last `consideration_set <consideration_set>` in scheduler that does not contain only
# # # learning-related nodes, ControlMechanism(s) or control-related ObjectiveMechanism(s);
# # # note: get copy of the consideration_set, as don't want to modify one actually used by scheduler
# # # Next, remove any learning-related nodes, ControlMechanism(s) or control-related
# # # ObjectiveMechanism(s) that may have "snuck in" (i.e., happen to be in the set)
# # # Then, add any nodes that are not learning-related or a ControlMechanism,
# # # and that have *no* efferent Projections
# # # IMPLEMENTATION NOTE:
# # # Do this here, as the list considers entire sets in the consideration queue,
# # # and a node with no efferents may be in the same set as one with efferents
# # # if they have the same dependencies.
# # Assign TERMINAL role to nodes that are last in the scheduler's consideration queue that are:
# # - not used for Learning;
# # - not ControlMechanisms or ObjectiveMechanisms that project to them;
# # - do not project to any other nodes.
# # FIX 4/25/20 [JDC]: MISSES ObjectiveMechanism BURIED IN LAST CONSIDERATION SET
# # First, find last `consideration_set <consideration_set>` in scheduler that does not contain only
# # learning-related nodes, ControlMechanism(s) or control-related ObjectiveMechanism(s);
# # note: get copy of the consideration_set, as don't want to modify one actually used by scheduler
# # MODIFIED 4/25/20 END
# endregion

# MODIFIED 4/25/20 NEW:
# FIX 4/25/20 [JDC]: NEED TO AVOID AUTOMATICALLY (RE-)ASSIGNING ONES REMOVED BY exclude_node_roles
# - Simply execlude any LEARNING_OBJECTIVE and CONTROL_OBJECTIVE that project only to ModulatoryMechanism
# - Simply exclude any LEARNING_OBJECTIVE and CONTROL_OBJECTIVE that project only to ModulatoryMechanism
# - NOTE IN PROGRAM ERROR FAILURE TO ASSIGN CONTROL_OBJECTIVE

# OUTPUT
Expand Down Expand Up @@ -4456,7 +4420,7 @@ def _determine_node_roles(self, context=None):
# self._add_node_role(node, NodeRole.OUTPUT)
# continue

# Assign OUTPUT if it is an `RecurrentTransferMechanism` configured for learning
# Assign OUTPUT if it is a `RecurrentTransferMechanism` configured for learning
# and doesn't project to any Nodes other than its `AutoassociativeLearningMechanism`
# (this is not picked up as a `TERMINAL` since it projects to the `AutoassociativeLearningMechanism`)
# but can (or already does) project to an output_CIM
Expand Down Expand Up @@ -4486,7 +4450,15 @@ def _determine_node_roles(self, context=None):
or (isinstance(p.receiver.owner, ControlMechanism) and not isinstance(node, ObjectiveMechanism)))
for p in node.efferents):
self._add_node_role(node, NodeRole.OUTPUT)
# MODIFIED 4/25/20 END

# If node is a Composition and its output_CIM has projections to self.output_CIM, assign as OUTPUT
# Note: this ensures that nested Comps that have both Nodes that project to Nodes in outer Composition
# *and also* legit OUTPUT Nodes, the latter are sufficient to make the nested Comp an OUTPUT Node
if (isinstance(node, Composition)
and any(proj.receiver.owner is self.output_CIM for
port in node.output_CIM.output_ports
for proj in port.efferents)):
self._add_node_role(node, NodeRole.OUTPUT)

# Assign SINGLETON and INTERNAL nodes
for node in self.nodes:
Expand Down Expand Up @@ -10459,7 +10431,7 @@ def output_values(self):
def get_output_values(self, context=None):
return [output_port.parameters.value.get(context)
for output_port in self.output_CIM.output_ports
if not self.output_CIM._sender_is_probe(output_port)]
if (not self.output_CIM._sender_is_probe(output_port) or self.include_probes_in_output)]

@property
def shadowing_dict(self):
Expand Down
36 changes: 18 additions & 18 deletions tests/composition/test_composition.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6296,21 +6296,21 @@ def test_unnested_PROBE(self):
comp = Composition(pathways=[A,(B, NodeRole.PROBE), C], name='COMP')
assert B.output_port in comp.output_CIM.port_map

params = [
( # id allow_probes include_probes_in_output err_msg
params = [ # id allow_probes include_probes_in_output err_msg
(
"allow_probes_True", True, False, None
),
# (
# "allow_probes_True", True, True, None
# ),
# (
# "allow_probes_False", False, False,
# "B found in nested Composition of OUTER COMP (MIDDLE COMP) but without required NodeRole.OUTPUT."
# ),
# (
# "allow_probes_CONTROL", "CONTROL", True,
# "B found in nested Composition of OUTER COMP (MIDDLE COMP) but without required NodeRole.OUTPUT."
# )
(
"allow_probes_True", True, True, None
),
(
"allow_probes_False", False, False,
"B found in nested Composition of OUTER COMP (MIDDLE COMP) but without required NodeRole.OUTPUT."
),
(
"allow_probes_CONTROL", "CONTROL", True,
"B found in nested Composition of OUTER COMP (MIDDLE COMP) but without required NodeRole.OUTPUT."
)
]
@pytest.mark.parametrize('id, allow_probes, include_probes_in_output, err_msg', params, ids=[x[0] for x in params])
def test_nested_PROBES(self, id, allow_probes, include_probes_in_output, err_msg):
Expand All @@ -6332,21 +6332,21 @@ def test_nested_PROBES(self, id, allow_probes, include_probes_in_output, err_msg

if not err_msg:
ocomp = Composition(name='OUTER COMP',
# node=[0,mcomp], # <- CRASHES DUE TO INFINITE RECURSION
# node=[0,mcomp], # <- CRASHES
nodes=[mcomp,O],
# nodes=[(mcomp, NodeRole.OUTPUT),O],
allow_probes=allow_probes,
include_probes_in_output=include_probes_in_output
)
ocomp.show_graph(show_cim=True, show_node_structure=True)
assert True
# ocomp.show_graph(show_cim=True, show_node_structure=True)

assert B.output_port in icomp.output_CIM.port_map
# assert B.output_port in mcomp.output_CIM.port_map
assert Y.output_port in mcomp.output_CIM.port_map
if include_probes_in_output is False:
assert len(ocomp.output_values)==3 # Should only be outputs from mcomp (C, Z) and O
assert len(ocomp.output_values)==4 # Should only be outputs from mcomp (C, Z) and O
elif include_probes_in_output is True:
assert len(ocomp.output_values)==4 # Outputs from mcomp (C and Z) and O (from B and Y)
assert len(ocomp.output_values)==6 # Outputs from mcomp (C and Z) and O (from B and Y)
# This tests that outputs from mcomp (C and Z) are included
# even though mcomp also projects to O (for B and Y)
else:
Expand Down

0 comments on commit 90b134b

Please sign in to comment.