Fixed bugs of converters in the case of that the range of variable is 0

[a-matsuo]

Summary

Fixes #1238

Details and comments

When the range of additional slack variables is 0, the InequalityToEquality and IntegerToBinary converters had unexpected behaviors.
This PR fixes that issue.

[t-imamichi]

How about showing a warning that an integer variable has 0-range?
E.g.,

qp = QuadraticProgram()
qp.integer_var(name='x', lowerbound=0, upperbound=0)
# -> Warning: integer variable 'x' has range 0.

[a-matsuo]

I think it does make sense. Shall I add the warning message?

[t-imamichi]

I'm OK. What do you think, @stefan-woerner and @woodsp-ibm?

[woodsp-ibm]

When adding a variable to QP the Variable class has this

        if lowerbound > self.upperbound:
            raise QiskitOptimizationError("Lowerbound is greater than upperbound!")

This seems to imply that lowerbound equal to upperbound is a valid Variable, whether it is 0 or some other number. In effect the variable is a constant value assuming at least one of the bounds is an inclusive value.

So this issue is fixing an issue in the conversion, not where the user did this but where a conversion would add a variable which would be a constant and have value 0 due to bounds. I am not sure where the problem arises with this 'constant'. It seems as an end user I am able to create such 'constants' via having the bounds the same. In one of the conversions such a constant is dropped in the other an error is raised.

On the warning suggestion is this some special case with 0 - or is it true for any with upper/lower bounds equal? Should this be a warning or should the check in Variable be changed to include equality such its an error. Maybe this equality is just an issue for these two convertors and it is fine for QP in general to have such Variables with only one possible value i.e. a constant. So I cannot say one way or another if raising a warning is the right thing to do - in my mind it depends on what is the expected behavior when bounds are the same and/or both are 0.

[t-imamichi]

I think lowerbound == self.upperbound is not a valid condition as a variable. Such variable should be given as a constant of the input model. So, it is an option to modify the condition as follows.

        if lowerbound >= self.upperbound:
            raise QiskitOptimizationError("Lowerbound is equal to or greater than upperbound!")

The conversion algorithm uses an assumption that the range of bounds is non-zero.

[stefan-woerner]

I think having lb == ub is valid. E.g. think about some higher level algorithms (decomposition etc.) that automatically update bounds and leverage a QUBO solver underneath, then that might happen (and is not necessarily handled specifically).

So I would not at a warning.
You can either let the converter replace it by a constant and introduce the value again later to the solution. But for transparency reasons, I would just handle it as is, i.e., have a single binary variable, that is forced to be 0.

[t-imamichi]

I see. Then, we maybe need another converter to substitute 0-range integer variables with constants. It is possible to implement it using QuadraticProgram.substitute_variables.

[stefan-woerner]

yes, we already have this functionality via substitute_variables. We can add such a converter on our backlog. For now, I would let the user handle this special case.

[t-imamichi]

OK. So, I think this pullreq is ready if the conflict is resolved. We need to make another converter IntegerToConstant as another pullreq.

[woodsp-ibm]

Can we fix the conflict here so this can be merged - I believe the PR is ready.

[a-matsuo]

Hi Steve, sorry, I had not merged the master. Now, the conflict should be solved. Ya, the PR is ready.