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

Wrong condition in the visualizer of alignments #401

Closed tomash87 closed 1 year ago

tomash87 commented 1 year ago

The condition if not move[0][0] == ">>" or move[0][1] == ">>": in https://github.com/pm4py/pm4py-core/blob/93fe16e628ede85318384ca1153a8dbc73c45fb9/pm4py/visualization/align_table/variants/classic.py#L77 is wrong, as it prevents the following condition in L79 https://github.com/pm4py/pm4py-core/blob/93fe16e628ede85318384ca1153a8dbc73c45fb9/pm4py/visualization/align_table/variants/classic.py#L79 to be satisfied. The condition in line 77 should be if not move[0][0] == ">>" and not move[0][1] == ">>": or if not (move[0][0] == ">>" or move[0][1] == ">>"):.

fit-alessandro-berti commented 1 year ago

Thanks for signaling the bug. We will release the fix in 2.7.1 / 2.8.0

fit-alessandro-berti commented 1 year ago

We fixed the issue in pm4py 2.7.1

tomash87 commented 1 year ago

I didn't realized this before, but the entire if-else structure in lines 77-82 is wrong: https://github.com/pm4py/pm4py-core/blob/35029b82a6b4c42949a790fa4e763fe2c3a82797/pm4py/visualization/align_table/variants/classic.py#L77 The fix from v2.7.1 has not fixed the actual error.

The move variable in lines 75-84 is a tuple of two elements, where the first element is log move and the second is model move. The conditions in if-else in lines 77-82 use two-layer addressing, e.g., move[0][0] and move[0][1]. This is purely wrong as it effectively selects the first and the second character in the log move. The first [0] index in all conditions in lines 77-82 have to be dropped.

This code really deserves better testing.

fit-alessandro-berti commented 1 year ago

Dear @tomash87

Sorry for these quality issues. We'll propose a proper change as soon as possible. Thanks for signaling

fit-alessandro-berti commented 1 year ago
image

Again sorry for the problems. The input was supposed to be different than the standard alignments output (you could obtain it by setting the parameter "ret_tuple_as_trans_desc" as "True"). In pm4py 2.7.2 it works on standard alignments outputs. Here, you see we also revised the coloring (light green = sync moves, violet = move on model, orange = move on log)

fit-alessandro-berti commented 1 year ago

Another example:

image