Seniority zero transformer

[jschuhmac]

Summary

First implementation of the SeniorityZeroTransformer (#46).
The implementation is based on to the ActiveSpaceTransformer and therefore the SeniorityZeroTransformer can be used in the same way.
Monkey patching is used to create the second quantization operators in the restricted formalism.

Details and comments

The SeniorityZeroTransformer transforms the one- and two-body integrals of the (standard) unrestricted Hartree-Fock method into the restricted formalism. In the restricted formalism the electrons are only treated in pairs, halving the number of qubits required, but at the cost of losing some accuracy by neglecting the single-electron excitations.

Building the second quantized operators based on one- and two-body integrals is currently only implemented for fermions (the second_q_ops() method for properties in qiskit_nature.properties.second_quantization.electronic returns FermionicOp). For the moment the creation of the operators in the restricted formalism is done with monkey patching.

Next to the integrals (stored in the ElectronicEnergy property) also the ParticleNumber property is transformed. The number of particles is left unchanged, only the second_q_ops method is updated to return an operator that counts two particles per excitation.

Example how to use the SeniorityZeroTransformer:

from qiskit_nature.drivers import Molecule
from qiskit_nature.drivers.second_quantization import ElectronicStructureDriverType, ElectronicStructureMoleculeDriver
from qiskit_nature.problems.second_quantization import ElectronicStructureProblem
from qiskit_nature.transformers.second_quantization.electronic import SeniorityZeroTransformer

molecule = Molecule(geometry=[["H", [0.0, 0.0, 0.0]], ["H", [0.0, 0.0, 0.8]]], charge=0, multiplicity=1)
driver = ElectronicStructureMoleculeDriver(molecule, basis="sto3g", driver_type=ElectronicStructureDriverType.PYSCF)

es_problem = ElectronicStructureProblem(driver, transformers=[SeniorityZeroTransformer()])

Discussion

There are still some points open for discussion:

• A VibrationalOp is used to implement the electron pair operators, but the VibrationalOp is not intended to be used in such a way.
• Instead of monkey patching @mrossinek suggested to use a Strategy.
• We only transform the ElectronicEnergy and the ParticleNumber property. Could this also be done for the other properties, AngularMomentum, Magnetization and ElectronicDipoleMoment?

[mrossinek]

A short FYI comment from my side:
The initial implementation was trying to be clever by using generator.send to inject the newly transformed property back into the iterated object. However, this does not work as intended when you want to remove properties because sending None will not overwrite the existing iterated object.

[mrossinek]

While this is indeed just a copy, I would leave this as such for the time being, because this code will be refactored slightly in the near future anyways 👍

[mrossinek]

This monkeypatching approach is currently a bit of a hack. An alternative to this approach is the use of a Strategy pattern as was already mentioned in the PR description. While this would make things slightly cleaner, it would have the downside of extracting all current Property.second_q_ops() implementations into a single "default" strategy.
A transformer which would affect this logic would then ship its own strategy which needs to overwrite the methods for all supported property types.

In the future, adding a new property will then cause all existing strategies to be extended by another implementation. Thus we have a NxM problem with N being the number of properties and M the number of strategies.

I am open to better suggestions of handling this 👍

[mrossinek]

Technically, hr_2 has the shape of 1-body terms but will be injected into a TwoBodyElectronicIntegrals object here. There is currently no better way of doing this unfortunately, because of the internal structure of the IntegralProperty and ElectronicIntegrals classes.
These will be refactored in the near future to resolve this temporary workaround.

[coveralls]

Pull Request Test Coverage Report for Build 2268361340

• 106 of 115 (92.17%) changed or added relevant lines in 4 files are covered.
• 1 unchanged line in 1 file lost coverage.
• Overall coverage increased (+0.1%) to 83.68%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
qiskit_nature/transformers/second_quantization/electronic/active_space_transformer.py	7	9	77.78%
qiskit_nature/transformers/second_quantization/electronic/seniority_zero_transformer.py	93	100	93.0%

Files with Coverage Reduction	New Missed Lines	%
qiskit_nature/transformers/second_quantization/electronic/active_space_transformer.py	1	91.79%

Totals
Change from base Build 2240694193:	0.1%
Covered Lines:	9281
Relevant Lines:	11091

---

💛 - Coveralls

[woodsp-ibm]

Should this be public in this class, I think it would be better annotated as private. As a function, at first sight, having self as the first parameter is confusing, but then I see above this is monkey patched into an class instance where self is the target of the monkey patching. Since I also see that monkey patching has already been commented on, hopefully something better is figured out in general.

[mrossinek]

What do you think about the strategy pattern suggestion? It is not perfect but the best idea I have had so far in order to remove the need for monkeypatching

[woodsp-ibm]

The monkey patching is really in effect just dynamically creating a subclasses of ElectronicEnergy etc. Could they be created as permanent sub-classes under the vibrational' side and just used where the transform changes it from an ElectronicEnergy in the electronic` side to an equivalent there with just the second_q_ops overidden (that is all I see monkey patched). Its always going to alter from one property type to another right.

[mrossinek]

@pbark and I were discussing with @jschuhmac offline, that the naming vibrational is more than bad in this scenario. We are using this operator in this scenario because we want to describe pairs of fermions which, together, behave like a boson. When we originally wrote the VibrationalOp, we voted against the name BosonicOp for reasons which I do not recall in all detail. However, now, I believe we need to revisit that naming choice.

All that said, I am actually wondering whether we want to make the SeniorityZero more of a first-class citizen by taking an approach of implementing two methods along the lines of to_fermionic_op() (for the current "default" behavior) and to_vibrational_op() (with an updated name depending on what we discuss above) in all properties directly.
Alternatively, we could even envision providing some sort of argument to second_q_ops() or to_second_q_op() (this needs to be unified, but this is another issue).
I am not sure whether this scales because I do not recall all of the details which the ParticleHoleTransformer will bring (i.e. does it affect only data or also the strategy) but I wanted to at least put this option up for discussion.

[woodsp-ibm]

I don't fully follow - as you say the transform makes the operator bosonic in nature by working with pairs, so I do not quite see how a to_fermionic_op applies. I was more thinking here that the properties were being transformed into something bosonic in nature and hence properties reflecting that so that the operator built would be bosonic. Or maybe you are thinking that properties have these methods for fermionic or bosonic and can implement one of the other as in your option to second_q_ops. Is there an option here though where some property could reasonably emit either?

My recollection of PH transform as it used to be was just a change in the data along with an energy extracted where the energy is the HF energy so that UCCSD first iteration will emit 0 once this is done.

[mrossinek]

Sorry, I meant that we could implement both methods (to_fermionic_op(), the current default, and to_bosonic_op(), or whatever we will call it). But I am not sure if this will make sense

[woodsp-ibm]

This code is dropping properties like DipoleMoment I guess. I think the main docstring should should state this so that the user is not left wondering why they have no dipole moment, and/or whatever else was dropped, in the result etc.

And I was wondering whether there should be a warn here too rather than simply silently discarding them. At present though since we don't have the user ability to select which of these auxiliary properties are added, I am not so sure as this would be constantly warning with nothing an end user can do about it.

[jschuhmac]

I have updated the docstring and the way how the unsupported properties are handled. The _transform_property method now returns None whenever it receives an unsupported property, and a warning is logged when a property is removed from the GroupedProperty.

[woodsp-ibm]

Since some properties are dropped, why not just warn and drop this too rather than make it an error. At least this way things would be a bit more future proof in that if additional properties are added for the problem that this will continue to work - though of course maybe it can be extended to better support such a property rather than dropping it.

[mrossinek]

Over time, this PR has gotten rather stale. This is mostly because the problems we hit during its implementation led to an improved code design reflected by the last 2 releases (0.5 and 0.6). However, in order to really implement this in the best way possible, we should address #1143 first.

However, for the time being, the priority of such an implementation is rather low. Thus, I will close this (now stale) PR.