Linear expansion pockets

[Arunavo71]

This pull request contains:

1. determine_linear_max_pockets for determing the maximal pockets when the expansion chain is linear.
2. An updated determine_max_pockets function, which should run marginally faster.
3. Tests for the functions.

[TimKam]

I added some remarks regarding aspects that can be improved or clarified.
While you are working on this, I can already take a preliminary look at how fast this runs.

[TimKam]

Can't this be a for loop? For loops are simpler than while loops so typically one should use for loops instead of while loops where possible.

[TimKam]

What does this loop do? It looks like it increments the index and executes the check for the if condition but nothing else. So I think it's best to wrap this into its own function and use a return instead of a break (and a for loop, that uses enumerate(list) such that you can use the index). This would also mean that you have to document that new function, which makes it easier to understand why you are doing this :)

[TimKam]

pockets or max pockets?

[TimKam]

Can you rename this to something more descriptive?

[TimKam]

Add a comment above this line describing what the code in the lines below does.

[TimKam]

In the function signature above it says that it returns a list of strings.

[TimKam]

use a more descriptive variable name

[TimKam]

use a more descriptive variable name

[TimKam]

Why this condition? Can you add a code comment?

[TimKam]

Can you add a comment explaining what this does?
Also modified is not a good name.

[TimKam]

Testing this empirically, I run into the following problems:

• Non-linear max pockets: machine runs out of RAM at graph size of ~16 already.

• Linear max pockets: list index is out of range:

--> 283   args = set(new_arguments(qbaf_initial,[qbaf_collection[collection_index]]))
    284   p_set = [set(x) for x in combinations(new_args, len(new_args)-1)]
    285   pocket_list = [list(x) for x in p_set if (args.issubset(x) == False)]

IndexError: list index out of range

[TimKam]

@Arunavo71 What about using this for the linear pocket algorithm:

for index, qbaf in enumerate(qbaf_collection):
    if not qbaf_initial.are_strength_consistent(qbaf, topic_argument_1, topic_argument_2):
      previous_qbaf = qbaf_collection[index-1] if index > 0 else qbaf_initial
      new_args_to_prev = list(new_arguments(qbaf_initial, [previous_qbaf]))
      intermediate_max_pockets = determine_max_pockets(previous_qbaf, [qbaf], topic_argument_1, topic_argument_2)
      return [pocket + new_args_to_prev for pocket in intermediate_max_pockets]
  return [list(new_arguments(qbaf_initial, qbaf_collection))]

1. Line 1 iterates through all QBAFs in the collection, which we assume are an ordered expansion chain.
2. Line 2 finds the first QBAF that breaks strength consistency/robustness.
3. Line 3 selects the previous QBAF, in which our inference is still robust.
4. Line 4 gives us a starting point for a pocket. Now, we only need to search between the current and the previous QBAF to find maximal pockets (which we do in Lines 5,6, or--if we are robust--we return all new arguments in Line 7).

This algorithm does not return all maximal pockets but it returns some, which is pragmatically what we need for explanations.
It scales without problems to QBAFs with hundreds of arguments, given my experiments with expansion chains of layered QBAFs. I also checked in a smaller experimental setup that it does not return max. pockets that the baseline algorithm would not return, either.

[Arunavo71]

Sure, let's use this updated code. Thank you. :)
I will also add some result stating that in these cases finding pocket is easy.

[TimKam]

I just noticed that the algorithm (and also previous variants) obviously does not guarantee maximality given the entire QBAF chain, i.e., it only gives us "local" maximality. This is important to mention.