Support Dict[str, OperatorBase] for aux_operators

[mrossinek]

What is the expected enhancement?

We should add support for Dict[str, Optional[OperatorBase]] structures to be passed as aux_operators into our algorithms.

Motivation

Currently, both the MinimumEigensolver (here) and the Eigensolver (here) interfaces only allow lists of operators to be passed to the aux_operators argument:

    @abstractmethod
    def compute_eigenvalues(
        self, operator: OperatorBase, aux_operators: Optional[List[Optional[OperatorBase]]] = None
    ) -> "EigensolverResult":

This causes very bad code quality in the application modules because we need to keep track of the order of the auxiliary operators, resulting in objectively bad code similar to this:

    for aux_op_eigenvalues in aux_operator_eigenvalues:
        if aux_op_eigenvalues is None:
            continue

        if aux_op_eigenvalues[0] is not None:
            result.num_particles.append(aux_op_eigenvalues[0][0].real)

        if aux_op_eigenvalues[1] is not None:
            result.total_angular_momentum.append(aux_op_eigenvalues[1][0].real)

        if aux_op_eigenvalues[2] is not None:
            result.magnetization.append(aux_op_eigenvalues[2][0].real)

        if len(aux_op_eigenvalues) >= 6 and q_molecule_transformed.has_dipole_integrals:
            _interpret_dipole_results(aux_op_eigenvalues, q_molecule_transformed, result)

For (hopefully) obvious reasons, we would like to improve this situation.

With the upcoming introduction of so-called Property objects in Qiskit Nature (qiskit-community/qiskit-nature#220, qiskit-community/qiskit-nature#263), we can naturally move to a Dict[str, Optional[OperatorBase]] structure to be passed into the algorithms instead of the current List (qiskit-community/qiskit-nature#263 (comment)).
This will arguably result in much more stable code in the application modules (speaking for Nature but I assume other modules would also be able to make use of this enhancement).

[mrossinek]

Another thought occurred to me: we should probably allow the dictionary values to also be a list:

Dict[
    str,
    Union[
        Optional[OperatorBase],
        List[Optional[OperatorBase]
    ]
]

[woodsp-ibm]

The original thinking is going to a dict instead of a List was that we create them at some level, which the final values are interpreted again at, but as the list progressed down the stack towards final execution at the algorithm some action(s) against the main operator may cause one or more aux ops to become 'invalid'. Eg symmetry reduction is where we encounter it as present where reducing/tapering the main operator by symmetry, if an aux ops does not commute with the symmetry then we should not measure it. As the list was an ordered list, to preserve the order a None was put in the operators place and a None in the resultant value - also a list in the same order. This means the originator has to deal with None as a result where the meaning is the operator could not be measured. Rather than this is was felt a dictionary, where we could simply drop the key/value pair if the aux operator needed to be discarded would be simpler all round - the originator still needs to deal with the result missing keys as no measurement could be taken.

In regards of having a key which points to a List, as per above, if any one of that List needs to be dropped then we are back in the same situation so to me it seems like a bad idea - if we really need nested structure - which seems to complicate things all round - then I think again it should be a Dict.

Whether a List or Dict coding the keys or indexes in many places as explicit values rather than having some Constant values declared somewhere and using is less that is poor coding anyway - the above sample could arguably be improved in that regard but it does not detract from the fact that having a Dict, where order is not the way we know what is what, but can be defined by the user and their choice of keys, is a better way to go.

[CisterMoke]

Hello, I am interested in picking up this issue as my first contribution. If I understand the issue correctly, the list of classes that should be updated are:

• NumPyMinimumEigensolver
• VQE
• QAOA
• NumPyEigensolver

Looking a bit further in the code it seems that the eigenvalues of the auxiliary operators for a certain eigenstate are returned as a NumPy array that preserves the order of the operators.
Which order should be chosen when aux_operators is a dict? Should I sort the keys or can I simply iterate over dict.values()?

[woodsp-ibm]

@CisterMoke I think the intent was that if a dictionary was passed in that a corresponding dictionary containing the results would be passed back so the keys are the same set. I.e. whatever the key was for the operator in the dictionary that was input, its measured expectation value in the output result would be in a dictionary under the exact same key. Hence the caller simply gets the results by key and there is no order involved.

[CisterMoke]

Hmm ok makes sense. I'm not immediately sure what possible side effects this could introduce but I'll start investigating that.

[mrossinek]

Thanks @CisterMoke for offering your help! What Steve wrote is correct: the intent would be to always access values by keys thereby removing any dependence on ordering. I am also not sure what side effects this will have, so please feel free to share any findings you have.

I also wanted to quickly point out what Steve wrote here:

In regards of having a key which points to a List, as per above, if any one of that List needs to be dropped then we are back in the same situation so to me it seems like a bad idea - if we really need nested structure - which seems to complicate things all round - then I think again it should be a Dict.

I absolutely agree with this (but somehow must have missed his reply previously). Essentially what this means is that the type I proposed above should really be more like this:

Dict[
    str,
    Union[
        Optional[OperatorBase],
        Optional[
            Dict[
                str,
                Optional[OperatorBase]
            ]
        ]
    ]
]

Note, that this could in theory be continued recursively (although I am not sure if that is something we would like to support).