List of functions and parameters that are not used

[apozas]

My plan here is to list all functions, objects, etc. that we create somewhere but then not used anywhere, in order to decide if we eliminate them or not.

• InflationSDP.generating_monomials_sym
• fast_npa.pad_with_zeros
• fast_npa.remove_zeros
• general_tools.apply_source_permutation_monomial
• general_tools.hypergraphs_are_equal
• general_tools.hypergraph_of_a_factor
• general_tools.reorder_according_to_known_semiknown_unknown
• general_tools.combine_products_of_unknowns
• general_tools.monomial_to_var_repr
• general_tools.change_to_representative_variables
• general_tools.objective_sym2index

[ecboghiu]

Hi, I will copy paste and add my comments:

• InflationSDP.generating_monomials_sym My idea here is to give to the user something friendly to see, as opposed to InflationSDP.generating_monomials which is a list of np.ndarrays. But it could also be removed. If the user wants to print out the generating set, they can also build it separately with .build_columns which returns also a list of symbolic operators.
• fast_npa.pad_with_zeros Safe to remove, relic from old code.
• fast_npa.remove_zeros Safe to remove, relic from old code.
• general_tools.apply_source_permutation_monomial This is currently not used, but we might need it. Right now, here: https://github.com/ecboghiu/inflation/blob/9102da55bc2f07d58765291aa0e324552a2f254a/causalinflation/quantum/general_tools.py#L328 we could plug in this function, making it easier to follow what is going on.
• general_tools.hypergraphs_are_equal Relic from old code, safe to remove, and we can recover it if we need it again.
• general_tools.hypergraph_of_a_factor Relic from old code, safe to remove, and we can recover it if we need it again.
• general_tools.reorder_according_to_known_semiknown_unknown Save to remove.
• general_tools.combine_products_of_unknowns This function should be used somewhere! Can you check that we are corrrectly recombining products of unknown variables into a single unknown variable? We might have removed this by mistake on Friday.
• general_tools.monomial_to_var_repr Safe to remove. Long term we don't want to encode information into arrays of rows with integer and monomial anyway, we should move to lists of monomials, and dictionaries mapping them to indices.
• general_tools.change_to_representative_variables Safe to remove. Same comment as before.
• general_tools.objective_sym2index Safe to remove. This functionality has been move to InflationSDP.set_objective().

[apozas]

Thanks for all the notes. I will be removing those that are safe to remove.

general_tools.combine_products_of_unknowns This function should be used somewhere! Can you check that we are corrrectly recombining products of unknown variables into a single unknown variable? We might have removed this by mistake on Friday.

Indeed, we removed its use when trying to fix #24, namely in this commit.

I have checked that optimization problems still work well (#32 was run after the commit had been done), but feasibility problems are failing. I'm opening a new issue with this problem in particular.

[apozas]

I have removed all the "safe to remove" functions in 605360f8defd57567b28eafedb7f138c17055a80. Also, as described in #34, I am being able to reproduce known results without using general_tools.combine_products_of_unknowns. I will wait a bit to see whether we find an instance where we need it.

[ecboghiu]

How about the monomial M_1 which factorises as <A_1_1_0_0_0> <B_2_0_2_0_0*B_2_0_1_0_0> <C_0_2_3_0_0*C_0_1_3_0_0>. This is a product of known unknown unknown and clearly the LPI constraints will not be correctly imposed. In this case, we need to find the index of M_2 = <B_2_0_2_0_0*B_2_0_1_0_0> <C_0_2_3_0_0*C_0_1_3_0_0> to impose the constraint a M_1 = <A_1_1_0_0_0> M_2.

I think you should use the function if we already have it written. However as I mentioned in some online meeting, and I write it here for the record, I would be careful and make sure that the factors <B_2_0_2_0_0*B_2_0_1_0_0> <C_0_2_3_0_0*C_0_1_3_0_0> are not in the representative form under inflation symmetries, as that might make them not factorise anymore when we combine them. For example, <B_2_0_2_0_0*B_2_0_1_0_0> can be simplified to a lexicographically smaller monomial by applying source swaps, namely <B_1_0_1_0_0*B_1_0_2_0_0>, and <C_0_2_3_0_0*C_0_1_3_0_0> can be simplified to <C_0_1_1_0_0*C_0_2_1_0_0>, and if simply join the representative representations into <B_1_0_1_0_0*B_1_0_2_0_0* C_0_1_1_0_0*C_0_2_1_0_0> this no longer factorises.

[apozas]

Note that not using the function was not a deliberate choice. If the code works without it, there is no reason to add it. In order to have a reason, we must have an explicit example that we see it failing. Please provide such example and we will fix it.

[ecboghiu]

I've added a test in https://github.com/ecboghiu/inflation/commit/4f6af2dfee508ba18a47077ac2b8f84cbdf70e03 which implements the example I gave; it fails as expected. I believe using that function will fix this. I've also modified some functions to allow me to code the test (there where some problems passing the output of build_columns() to generate_relaxation().

The test is here: https://github.com/ecboghiu/inflation/blob/4f6af2dfee508ba18a47077ac2b8f84cbdf70e03/test/test_sdp.py#L317

[apozas]

I saw that same behavior, and in order to fix it I decided to undergo the refactoring of the core of generate_relaxation. In the process, I surfaced a new potential issue that is currently making the test fail, although its initial motivation is fixed. I will open a new issue on this, because it may need some discussion in order to understand properly what's going on.

[ecboghiu]

In https://github.com/ecboghiu/inflation/commit/26fa1f0f8595f702eb2165c9b96c4a0e81c9050c I've removed combine_products_of_unknown from general_tools and wrote a much shorter version within InflationSDP. Now all the tests are passing! :)

[apozas]

Alright, so I think that for the time being we can consider this issue closed :)