Open falcaopetri opened 4 years ago
Hi.
I think the following logic may be related to "max length exploitation" for the Dangling Operator. I also feel that this may be a bug that incorrectly filters out candidate rules. The problem seems to occur only when mining constants, so it's hard to spot.
The above code's idea seems to be that "the last atom added to a rule should not be generated by the dangling operator since the resulting rule will definitely be not-closed". Nonetheless, this ignores the possibility of applying the instantiation operator and make the rule closed.
Indeed, the code above seems to contradict the check done by the dangling operator itself:
0k, now I get it.
The above code's idea seems to be that "the last atom added to a rule should not be generated by the dangling operator since the resulting rule will definitely be not-closed". Nonetheless, this ignores the possibility of applying the instantiation operator and make the rule closed.
The highlighted afirmation is wrong. Instantiation operator has already been applied to the dangling's output and is at temporalOutputMap.get("instantiated")
. The code I suggested to remove is just avoiding adding a rule which has max size and is surely unclosed (because of the newly added dangling variable). If added, the rule will be shortly filtered by AMIE's output check, and the final output will be the same.
I will drop PR #44 since it has already incomporated this wrong understanding.
Nonetheless, the modification to the dangling's exploitMaxLengthOption
is still valid, since it:
query.getOpenVariables().size() == 0
)query.getOpenVariables().size() > 1
). This skips the application of the dangling operator to rules such as ?a <r1> ?f => ?a <r2> ?b
.I think I've finally found the real culprit for the performance boost shown in #46. This is MiningAssistant.getInstantiatedAtoms
:
Note that if exploitMaxLengthOption
is set, then... max length is not exploited (because the condition checked will be true).
Therefore, my proposed fix to this issue is: the current PR #46 + fixing the code above. The PR is still useful since it discards rules that would be unnecessarily enqueued.
Hi Antonio,
Thanks for this nice inquiry. I agree with your analysis and I vote for merging your PR into AMIE's main branch. I am waiting for @lajus approval.
Best, Luis
Hi.
Maximum rule length is an interesting pruning technique introduced in AMIE+. In "Fast rule mining in ontological knowledge bases with AMIE+", we can read:
Summing up, we have that:
I guess that dangling operator can be safely modified to consider if there are more than one non-closed variable, just as the instantiation operator.
Indeed, the dangling operator will:
Therefore, it can close at most one variable. The newly added variable may be closed by instantiation operator, but the previously opened variables cannot.
Best, Antonio.