Removing unnecessary features and functions

[apozas]

In my cleanup of the code in the main branch (see here for all the changes in InflationSDP.py), I have found two things that I think that you can safely implement in your codes as well. I am referring to removing the argument positive_vars from the input to the solver (see e0630bdbfc93fe6427922bfd562825c4a8e5cc8e), and to removing the generation of InflationSDP.substitutions (see fccda1f91ead1de31b5cd07dcaf05db68b09a552).

For the former, the reason is that the same information is already present in the argument var_lowerbounds (see that in InflationSDP.generate_relaxation() the dictionary that is fed as var_lowerbounds is populated with the corresponding bounds of the positive variables). There is however a piece of code (removed in e0630bdbfc93fe6427922bfd562825c4a8e5cc8e as well) in which, if var_lowerbounds and positive_vars are not consistent, positive_vars takes precedence. While the idea makes sense, the way it is implemented is not good coding practice: the code is modifying the user's input without letting them know, and in a function that is not supposed to operate with the input (note that the goal of solveSDP_MosekFUSION should be just to parse and send the problem to the solver, whatever this problem is). I agree that we should have some sort of consistency check, but this should be inside InflationSDP, once we add the pieces for properly dealing with lower and upper bounds.

For the latter, well, the motivation is that we do not use them any more, anywhere. I wanted to raise the point mainly as a note to my future self that we will probably want to find a way of sanitizing the input to set_objective that works without the substitutions. For now, if a monomial is not found a message suggest that you look for its correct form in InflationSDP.monomial_list (see 4cdd02d4a502fdc1c10612dfc0d67a8532120ad2).

I wanted to let this here to know if there is anything I missed in the way, and to let you know that you probably want to implement these changes in your branches.

[ecboghiu]

In the atomic-complete branch in the process of removing ncpol2sdpa I removed the need to use substitutions to sanitise the intput to set_objective. See here: https://github.com/ecboghiu/inflation/blob/5bf3e4b0695cc2318c158539377d0968b7269f39/causalinflation/quantum/InflationSDP.py#L342 I also use orbits to look for representative versions of the sanitised-brought-to-canonical-form inputs, and only if they are not found, I use to_representative. I think you can safely accept these changes into the main branch.

[apozas]

Yes, that is a good idea. I just pushed the commit, which I checked it works by feeding the objective of test.test_sdp.TesdSDPOutput.test_CHSH as A0*(B0+B1)+(B0-B1)*A1 instead, and confirming that the test runs correctly. Moreover, instead of using orbits, I use _var2repr, which has the same functionality but the keys are the string-format names (which makes me think, does your code work in the current status? At least in the main branch, both keys and values in the orbits dictionary are integers).

[ecboghiu]

No, in the other branch I use the name representation for the dictionaries. However, I have taken care to modify all the places where indices were used to not need them. The only advantage I can think of is that perhaps comparing integers is faster than strings, but I don't have evidence that the speed gains from this are worth the hassle of keeping around this representation.