causalincentives / pycid

Library for graphical models of decision making, based on pgmpy and networkx
Apache License 2.0
96 stars 13 forks source link

Clarify `MACIDBase.make_chance` #37

Closed edlanglois closed 3 years ago

edlanglois commented 3 years ago

I don't understand the logic of MACIDBase.make_chance: (older version before I changed some names)

if node in self.all_decision_nodes:
    agent = self.whose_node[node]
    self.agent_decisions[agent].remove(node)
    self.whose_node.pop(node)
elif hasattr(self, "cpds") and node not in self.all_decision_nodes:
    pass
elif not hasattr(self, "cpds"):
    raise Exception("The (MA)CID has not yet been parameterised")

without the redundant checks this becomes

if node in self.all_decision_nodes:
    agent = self.whose_node[node]
    self.agent_decisions[agent].remove(node)
    self.whose_node.pop(node)
elif hasattr(self, "cpds"):
    pass
else:
    raise Exception("The (MA)CID has not yet been parameterised")

The exception is raised only if you pass a non-decision node and cpds aren't set yet? Shouldn't it be an error to pass a non-decision node at all? And what is the reason for failing if the graph isn't parameterised?

Jamesfox1 commented 3 years ago

Yes this should throw an error whenever it's passed a non-decision node. I have made the change!