Conversion performance improvement

[Sofranac-Boro]

Summary

The purpose of this merge request is to find easy wins in improving the performance of converters.

Details and comments

I ran some tests to see where the bottleneck in conversion lies. I used the enlight11.mps file, and tried to convert it to QUBO. On my machine, this is the breakdown of timings in seconds per the conversion step for this instance:

reading in mps file:  0.011665105819702148
translating:  0.008737564086914062
<class 'qiskit_optimization.converters.linear_inequality_to_penalty.LinearInequalityToPenalty'>  converter:  0.014148950576782227
<class 'qiskit_optimization.converters.inequality_to_equality.InequalityToEquality'>  converter:  0.01540994644165039
<class 'qiskit_optimization.converters.integer_to_binary.IntegerToBinary'>  converter:  0.09988164901733398
<class 'qiskit_optimization.converters.linear_equality_to_penalty.LinearEqualityToPenalty'>  converter:  10.31824779510498
<class 'qiskit_optimization.converters.flip_problem_sense.MaximizeToMinimize'>  converter:  1.1920928955078125e-05
Total conversion time:  10.447956562042236

By far the most time is spent in the LinearEqualityToPenalty converter. Out of ~10.3 seconds spent in this converter, ~9.3 seconds are spent in the _coeffs_to_dok_matrix function while processing the quadratic coefficients of the resulting QUBO.

This function first builds the QUBO matrix out of a dictionary of coefficients in full form (both upper and lower triangular elements are added), and then transforms it into an upper diagonal form via matrix operations. As these operations can be expensive, the proposed change simply directly builds the matrix in upper triangular form, avoiding several intermediate steps

In the following table, the conversion time is compared before and after the improvements for a few instances:

Instance	old	new
enlight11.mps	10.6	6.6
mzzv11.mps	32.3	28.5
acc-tight5.mps	3.8	3.1
air04.mps	82.7	53.6
iis-glass-cov.mps	21.5	18.0
f2gap40400.mps	0.7	0.4
cod105.mps	13.3	11.0

✅ I have added the tests to cover my changes.
✅ I have updated the documentation accordingly. [nothing to be updated]
✅ I have read the CONTRIBUTING document.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
2 out of 3 committers have signed the CLA.

✅ woodsp-ibm
✅ Sofranac-Boro
❌ Boro Sofranac

---

Boro Sofranac seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[Sofranac-Boro]

Does anyone have any idea what could cause python3.8 to fail the QAOA test, while other python versions work? Maybe some of you had similar experiences before? Here's what I know:

• tried running this test with python3.8 locally, it passes without problems.
• All other python versions work fine. Also 3.8 passes on windows, but not on mac or ubuntu.
• The offending test does indeed call the converter under the hood, which was touched by this MR. I double checked that the result of the conversion in the test is identical:

minimize 2*x0^2 + 0.5*x0*x1 + 0.5*x1^2 + 2*x0 + 2*x1 (2 variables, 1 constraints, 'op_ip1')
vs
minimize 2*x0^2 + 0.5*x0*x1 + 0.5*x1^2 + 2*x0 + 2*x1 (2 variables, 1 constraints, 'op_ip1')

The failure happens because the qaoa finishes with an suboptimal solution [1,1] (objective val 7) while cplex finds the optimum [0,2] (objective val 6)

[woodsp-ibm]

It does seem rather a puzzle as to why CI is failing here. Locally on a linux system I created a new env with Python 3..8.18, as its using here, and locally modifying the QuadraticExpression code the same the test works - whether I run it the same as done in CI, i.e using stestr, via make test or do it manually individually. So locally I was not able to reproduce the failure either. I can only assume that my env is not exactly the same, indeed Windows CI here uses a conda env so its not quite the same either and the libraries are a bit different for that platform too anyway.

One could try and put some temporary prints to come out in the log in an attempt to figure things out - but I can imagine this not being very productive. Locally I printed the coeffs built during the test that fails to those built the former way and looking over things they seemed the same - but the test passed for me anyways.

I could see 2 ways to proceed as-is with further trying to figure why it fails here - simply accept that for 3.8 it works, based on the fact that tests run outside locally work, and augment the test that is failing to skip it under Python 3.8, leaving whatever mystery is causing the failure be and that logic untested under 3.8 by CI. The other would be to change the code so it conditionally goes down the new, more performant, code path only for Python 3.9 or above, and for 3.8 therefore still does it as its currently done. That would still have it run and pass (as it does now hopefully!) in CI but a user a would not get the speed up under 3.8. Qiskit has been supporting Python versions until their EOL - for 3.8 thats 14th Oct 2024, so 8 months away where shortly thereafter I would imagine dropping 3.8 support anyway not long after Qiskit does so. I guess I like to see stuff tested so I would more lean to the 2nd option personally rather than assume it carries on working, based on having seen that at this moment it runs/passes locally. Either way still leaves the mystery unsolved but without being able to reproduce it locally it seems it may have to remain a mystery. And of course put some comment at the location of whatevers changed to workaround this about the CI failure and why things look like they do.

[coveralls]

Pull Request Test Coverage Report for Build 8298754055

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

• For more information on this, see Tracking coverage changes with pull request builds.
• To avoid this issue with future PRs, see these Recommended CI Configurations.
• For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

• 6 of 10 (60.0%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage decreased (-0.07%) to 92.959%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
qiskit_optimization/problems/quadratic_expression.py	6	10	60.0%

Totals
Change from base Build 7890111189:	-0.07%
Covered Lines:	4515
Relevant Lines:	4857

---

💛 - Coveralls