Property Finalization

[mrossinek]

Summary

Closes #264
Closes #148

Details and comments

Task list partially carried over from #263

• direct integration of DriverResult with drivers (i.e. do not return QMolecule as intermediate storage container)
• deprecation of BaseProblem.molecule_data* properties
• implementation of HDF5 interface (storing and loading similar to QMolecule) (cf. HDF5 integration for the Property framework #302)
• implement Property.log()
• release note
• tutorial (if time permits) [WIP]
• documentation updates:
  • Modular Driver Result #263 (comment)
  • Modular Driver Result #263 (comment)
  • generally double check correct contents and rendering

[mrossinek]

Since I cannot comment on a changed binary file:
The file test/transformers/second_quantization/electronic/LiH_sto3g_reduced.hdf5 had to be updated because the stored QMolecule in that file was missing its num_molecular_orbitals attribute.

[mrossinek]

This still needs to be cleaned up. In order for that to happen I need to figure out how to properly deprecate the BaseProblem.molecule_data*..
@woodsp-ibm @manoelmarques do you have some ideas on how to proceed with that?

Since they are part of the public API I am unsure if I can just change/delete them. They won't exist in their format any longer and they were really just auxiliary access points to the driver results (which are now Property objects).
In #263 I added the BaseProblem.roperties_transformed attribute as a temporary workaround. Ideally I think we should have something like this:

class BaseProblem:
    @property
    def driver_result(self) -> GroupedProperty:
        return self._driver_result

    @property
    def transformed_driver_result(self) -> GroupedProperty:
        return self._transformed_driver_result

Please let me know your opinions 🙂

[manoelmarques]

You can’t delete the properties if you intend to deprecate. Is it possible to get the molecule data by other means to keep the properties or is it lost ?

[mrossinek]

molecule_data used to be QMolecule or WatsonHamiltonian. Since these are now deprecated, they will no longer occur within the stack, if a user uses one of the new drivers in qiskit_nature.drivers.second_quantization.
I could deprecate the properties and change their type to be Optional[...] but I am not sure if that is allowed? [Technically they probably should have been that though, because the __init__ sets them to None...]

And to be clear: the deprecated drivers are still supported by this stack but right now they will not store these properties (which I could change ofcourse) 👍

[woodsp-ibm]

It would be nice if using things with the existing drivers, for users that have code to what is currently released, works the same albeit deprecated. Switching to the newer drivers then other code for properties can come about. At that level things can be more separated - in the problem I guess it has the unenviable task of, in the interim, allowing the old way to work and supporting the new way too if thats indeed doable.

[mrossinek]

I have updated the code and we now have the following:

• BaseProblem.molecule_data* are deprecated in favor of BaseProblem.grouped_property*

We have some internal logic as follows:

Legacy Driver + Legacy Transformers

Works as expected. Deprecation warnings will be raised by those classes themselvers.
The following properties will be set:

• molecule_data
• molecule_data_transformed
• grouped_property
• grouped_property_transformed

Legacy Driver + New Transformers

Works. A UserWarning will be raised, suggesting to look into the new drivers.
The following properties will be set:

• molecule_data
• molecule_data_transformed (remains None)
• grouped_property
• grouped_property_transformed

New Driver + Legacy Transformers

Impossible! A QiskitNatureError will be raised.

New Driver + New Transfomrers

Works as expected.
The following properties will be set:

• molecule_data (remains None)
• molecule_data_transformed (remains None)
• grouped_property
• grouped_property_transformed

Finally, if a user mixes legacy and new transformers, a QiskitNatureError will be raised, too.

[mrossinek]

FYI: #302

[mrossinek]

I am marking this as ready for review. Early next week, I am going to tackle the outstanding tasks:

• reno
• documentation completion + double checking
• tutorial completion

[woodsp-ibm]

In the notebook this warning message is emitted - as we use the default value for display_format. As this message is only emitted when we rely on the default can we explicitly pass in the value we want (maybe we need to change it internally to sparse later but I think thats better than having warnings come out by default from our internal use of our code)

/home/runner/work/qiskit-nature/qiskit-nature/qiskit_nature/properties/second_quantization/electronic/integrals/electronic_integrals.py:218: UserWarning: The default value for display_format will be changed from 'dense'to 'sparse' in version 0.3.0. Once that happens, you must specifydisplay_format='dense' directly.

(PS. I fixed for the formatting of the message in #330)

[woodsp-ibm]

In the notebook ParticleNumber prints out this

ParticleNumber:
        4 SOs
        1 alpha electrons: [1. 0.]
        1 beta electrons: [1. 0.]

I had to look at the code to figure what the [1. 0.] was for - with the example it looks not so far from the tuple that is used to define the num particles (1, 1) given the lengths are the same. I see now its the orbital occupation. It might be helpful to say a little more e.g. something like this perhaps, though I agree its a bit verbose; maybe there is a better way to format it and say what things are.

ParticleNumber:
        4 SOs
        1 alpha electrons:
              orbital occupation: [1. 0.]
        1 beta electrons:
             orbital occupation: [1. 0.]

[woodsp-ibm]

In looking at the strings, particularly the integrals, should we be truncating the output by default, like numpy does with its arrays. You example in the notebook is small. By the time the integrals are more real-world it seems the formatted string is going to be huge. Having a summary like the size/shape and then some of content seems more practical. With access to data it can be printed anyway the user wants, but the str here should be a useful summary/overview of that data - given sizes of things dumping out all the data seems too much.

[mrossinek]

I have added a truncation method similar to what we did for the FermionicOp. However, since we are explicitly listing the matrices as a sparse list, this truncation sets the number of entries to be displayed (i.e. the number of lines for each matrix) rather than limiting the total number of chars to be printed. I think this makes the most sense.

All changes implemented.