pm4py / pm4py-core

Public repository for the PM4Py (Process Mining for Python) project.
https://pm4py.fit.fraunhofer.de
GNU General Public License v3.0
722 stars 286 forks source link

Problem with Heuristics Miner: Key Error when getting heuristics net, depending on threshold #71

Closed carvalhomb closed 5 years ago

carvalhomb commented 5 years ago

Trying to use the Heuristics Miner functionality, I came across a KeyError when trying to get an heuristics net. The error depends on the dependency threshold set.

Code to reproduce the error below, with (zipped) example log attached.

from pm4py.objects.log.importer.csv import factory as csv_importer
from pm4py.objects.conversion.log import factory as conversion_factory

event_stream = csv_importer.import_event_stream("testlog.csv") 
log = conversion_factory.apply(event_stream)

# This works
heu_net = heuristics_miner.apply_heu(log, parameters={"dependency_thresh": 0.99})
gviz = hn_vis_factory.apply(heu_net)
hn_vis_factory.view(gviz)

# This gives KeyError
heu_net = heuristics_miner.apply_heu(log, parameters={"dependency_thresh": 0.9})
gviz = hn_vis_factory.apply(heu_net)
hn_vis_factory.view(gviz)

testlog.zip

Luttik commented 5 years ago

I am also having this KeyError exception. The offending method (for me) is calculate_loops_length_two in the file pm4py.objects.heuristics_net.node.py. Where dfg_matrix[n1][n2] throws the error.

Luttik commented 5 years ago

After browsing through the source code I found that dfg_matrix is used in an unsafe manner throughout the project.

Luttik commented 5 years ago

To fix this I suggest indexing dfg_matrix with tuples so dfg_matrix[n1,n2] then you can either fix this by calling dfg_matrix.get((n1, n2), 0) to safely get the values or you can use collections.abc.MutableMatrix to overwrite __get__ so you can still call dfg_matrix[n1, n2] and get a default value if none is present.

Javert899 commented 5 years ago

Thank you both for signaling the bug and the suggestions

I will try to solve the issue and release an hotfix release before Monday :)

Luttik commented 5 years ago

@Javert899 That sounds great. Thanks.

Javert899 commented 5 years ago

The signaled issue was corrected in hotfix release 1.1.10 (docker images are being rebuilt from now)

Thanks again for signaling

Luttik commented 5 years ago

@Javert899 Great. It seems to be working 👍

carvalhomb commented 5 years ago

Excellent, for me it is also working :) Closing the issue.