jpmorganchase / QOKit

QOKit
https://www.jpmorgan.com/technology/applied-research
Apache License 2.0
52 stars 23 forks source link

Unify `precomputed_diagonal_hamiltonian` and `precomputed_costs` #34

Closed HaoTy closed 8 months ago

HaoTy commented 10 months ago

Currently, there are two arguments related to precomputed energies in get_qaoa_objective(). precomputed_diagonal_hamiltonian is passed into the simulator but not used for get-objective functions after the simulation, except when precomputed_costs is None. However, judging from problem-specific get-objective functions, this exception is never the case, and even if so, it would produce incorrect results. Having two similar arguments leads to ambiguity and the duplication of the energy vector, whose size is exponential in the number of qubits.

rsln-s commented 9 months ago

TODOs:

  1. Confirm that in LABS the normalization of E = -N**2/(2*F) can be moved from 198 to 200. Make the change and see what tests fail.
  2. Figure out where the distinction between diagonal operator in QAOA circuit and the cost vector used to compute the expectation is present in fur across simulators. The logic should always the same (i.e. if costs == None, then use diagonal as costs).
  3. Is constrained case (XY mixer) handled correctly?
  4. If above succeeds, keep only precomputed_costs in get_qaoa_objective() and remove precomputed_diagonal_hamiltonian.
rsln-s commented 9 months ago

To correct the point above, note that this fix is incorrect.

  1. Confirm that in LABS the normalization of E = -N**2/(2*F) can be moved from 198 to 200. Make the change and see what tests fail.

To understand why this works, note that qaoa_objective = <\psi | C |\psi>. Since C = -(N**2) / (2 * y) - o, so we can by linearity write <\psi | C |\psi> = <\psi| -(N**2) / (2 * y) | \psi> - o = (-(N**2) / 2) * <\psi| 1 / y | \psi> - o

However <\psi| 1 / y | \psi> = \sum_i \alpha_i**2 * (1/y_i) != 1 / (\sum_i \alpha_i**2 y_i). So for LABS, we must keep the distinction for backward compatibility.

The solution is as follows:

  1. We make precomputed_diagonal_hamiltonian the required one. We add the following error:
    if precomputed_diagonal_hamiltonian is None and precomputed_costs is not None:
    raise ValueError("precomputed_diagonal_hamiltonian should be passed instead of precomputed_costs")

    This is to prevent logic around like 177-179 from breaking.

  2. In MaxCut, portfolio optimization etc. we only pass precomputed_diagonal_hamiltonian.
HaoTy commented 9 months ago

PO will also break as there's a scale difference between precomputed_diagonal_hamiltonian and precomputed_costs. Since the primary goals for unifying the two are to reduce memory footprint and centralize the computation in fur with the user-specified backend, I think a reasonable solution is to pass in a function that takes precomputed_costs and calculates precomputed_diagonal_hamiltonian (or vice versa). This function should be able to deal with input from different backends (which should be quite straightforward), and an additional wrapper can be added in the mpi simulator to ensure this function is called exactly once for each gpu.

rajgane07 commented 9 months ago

@HaoTy - Can we have call to discuss your idea?

rsln-s commented 9 months ago

As discussed with @rajgane07 on the call, we can stick to a simpler rescaling solution for portfolio optimization as it's only a linear transformation. Passing a conversion function as an argument for get_qaoa_objective creates significant engineering challenges for c and mpi simulators, and only improves the memory for the LABS use case. The other (including future) use cases will either only include a linear transformation or no transformation at all, avoiding this memory overhead. In this way, LABS is somewhat pathological.

For PO, we can change the code here as follows:

    return get_qaoa_objective(
        N=N,
        p=p,
        precomputed_diagonal_hamiltonian=po_problem["scale"] * precomputed_energies,
        precomputed_optimal_bitstrings=precomputed_optimal_bitstrings,
        parameterized_circuit=parameterized_circuit,
        parameterization=parameterization,
        objective=objective,
        simulator=simulator,
        mixer="xy",
        initial_state=sv0,
        n_trotters=T,
    ) / po_problem["scale"]

Thank you, @HaoTy, for you very valuable feedback which we very much appreciate! And also for pointing out the problem with the PO function.

rsln-s commented 9 months ago

As @HaoTy pointed out, the above solution does not handle the overlap correctly... The simplest fix is to add rescaling only when the objective is expectation. See rough idea for the code below. Perhaps there are more elegant solutions possible.


    def scale_result(f):
        def rescaled_f(*args):
            if objective == "expectation":
                return f(*args) / po_problem["scale"]
            elif objective == "expectation and overlap":
                res = f(*args)
                return res[0] / po_problem["scale"], res[1]
            else:
                assert objective == "overlap"
                return f(*args)
        return rescaled_f

    return scale_result(get_qaoa_objective(
        N=N,
        p=p,
        precomputed_diagonal_hamiltonian=po_problem["scale"] * precomputed_energies,
        precomputed_optimal_bitstrings=precomputed_optimal_bitstrings,
        parameterized_circuit=parameterized_circuit,
        parameterization=parameterization,
        objective=objective,
        simulator=simulator,
        mixer="xy",
        initial_state=sv0,
        n_trotters=T,
    )) 
rajgane07 commented 9 months ago

@rsln-s @HaoTy Portfolio Optimization fails after removing precomputed_costs for qiskit simulator, "ValueError: precomputed_objectives or terms are required when using the expectation objective" We manage in Maxcut, by passing the terms if cuts is None. Similarly for portfolio is there a way to calculate terms? If yes, can you share the code or pseudo code to incorporate it.