Fix a bug in grover optimizer

[t-imamichi]

Summary

I found a tricky code in GroverOptimizer and this PR fixes it.

Details and comments

In GroverOptimizer.solve it computed the objective function value of the optimal values in the following line.
https://github.com/Qiskit/qiskit-aqua/blob/e13728e6cdfdb1438a7d2d11d856622c90ce328b/qiskit/optimization/algorithms/grover_optimizer.py#L263
However, this fval is not the optimal function value of the original problem because problem_ is modified from the original one as follows.

• the objective function is multiplied by -1 if maximization
  https://github.com/Qiskit/qiskit-aqua/blob/e13728e6cdfdb1438a7d2d11d856622c90ce328b/qiskit/optimization/algorithms/grover_optimizer.py#L160
• the constant value of the objective function is iteratively modified
  https://github.com/Qiskit/qiskit-aqua/blob/e13728e6cdfdb1438a7d2d11d856622c90ce328b/qiskit/optimization/algorithms/grover_optimizer.py#L198

As a result, fval computed from problem_ is not what we expected. However, LinearEqualityToPenalty happens to correct the fval in the following line. So, it passes the unit tests.
https://github.com/Qiskit/qiskit-aqua/blob/e13728e6cdfdb1438a7d2d11d856622c90ce328b/qiskit/optimization/converters/linear_equality_to_penalty.py#L194

I modified the code so that OptimizationResult(x=opt_x, fval=fval, variables=problem_.variables) in the following line becomes the optimal solution to the original problem to improve the consistency of the code.
https://github.com/Qiskit/qiskit-aqua/blob/e13728e6cdfdb1438a7d2d11d856622c90ce328b/qiskit/optimization/algorithms/grover_optimizer.py#L264

[stefan-woerner]

@t-imamichi good catch!

[stefan-woerner]

@t-imamichi could you make sure the tests are covering this?

[t-imamichi]

Because the fval is hidden in the solve method, we need to copy the fval as part of GroverOptimizerResult. Perhaps, we have to do a similar stuff for other optimizers too if they use any converters internally. Let me check other optimizers. By the way, this bug is found as part of #1196.

[t-imamichi]

The other optimizers don't have a similiar issue. So, we need to take care of Grover optimizers.

If we check the integrity of fval of problem_ and fval of problem as part of unit tests, we need to copy fval of problem_ to a new entry of GroverOptimizationResult, e.g., GroverOptimizationResult.fval2. But, users don't need it for applications because GroverOptimizationResult.fval == GroverOptimizationResult.fval2 is expected. So, I'm wondering what is the best way.

[t-imamichi]

@stefan-woerner How about continuing to hide the internal fval? It might be complicated to add anther fval to OptimizationResult.

[woodsp-ibm]

@t-imamichi Since there exists raw_results, and also its easy enough to add a new field to the GroverOptimizationResult, I am wondering why it seems so hard to return this internal fval, if that is what is wanted. Outside of unit tests do we ever assert fval and fval2 match? If they did not is it useful to the end user to have this internal fval so they can check/see any mismatch - I guess some explanation might be needed around these properties in that respect so an end user can understand this if its meaningful to inform them of the internal fval.

[stefan-woerner]

@t-imamichi I think @woodsp-ibm is right, the raw_results should give access to the value for the modified problem, shouldn't it? The raw_results should actually provide the offset and the intermediate results, such that you could reconstruct the original problem at any stage.

[t-imamichi]

I agree that adding offset and intermediate results is informative, but I have a doubt of the internal fval because it is essentially identical with the external fval. The internal fval is not necessary to reconstruct the original problem at all because the external fval is same.

But, I add the internal fval in the following commits. If you want to add more intermediate values to GroverOptimizationResults, please let me know.

[t-imamichi]

I added intermediate_fval and threshold as part of GroverOptimizationResult and added a unit test using it.