Add PeriodicCollapser

[lukasturcani]

Co-authored by: @andrewtarzia
Issues addressed: #282, #278

This PR adds the following features:

• A new Optimizer, PeriodicCollapser, which can be used
  with periodic COF topology graphs.
• A subclass of ConstructionResult,
  called PeriodicConstructionResult, which additionally
  holds periodic information about the constructed molecule.
• A new initializer for ConstructedMolecule,
  init_from_construction_result. This new
  initializer can be used when the user calls
  TopologyGraph.construct() directly, and means that they
  can get the ConstructionResult and the corresponding
  ConstructedMolecule, without having to run
  construction twice.

This PR also deprecates the method TopologyGraph.get_periodic_info(),
use PeriodicConstructionResult.get_periodic_info() instead.

There were a couple of changes which needed to be made to add the
PeriodicCollapser. The main issue was that the PeriodicCollapser
needs to be able to modify the lattice constants, so that the
PeriodicInfo returned by
PeriodicConstructionResult.get_periodic_info(), has the
lattice constants shrunk by the same amount the
ConstructedMolecule is. In order to do this, the ConstructionState
and GraphState were extended to allow modification of the
lattice constants via with_lattice_constants() and retrieval of the
lattice constants via get_lattice_constants().

The PeriodicInfo is now being returned by the
PeriodicConstructionResult rather than by the TopologyGraph
because it avoids needing to run the construction twice, if the
user wishes to get the ConstructedMolecule and the
PeriodicInfo. If the PeriodicInfo was returned by the
TopologyGraph, TopologyGraph.get_periodic_info() would
have to run a construction. This is because it is only after the
construction that the value by which the PeriodicInfo
should be shrunk by is known. If the user wishes to get both
the PeriodicInfo and the ConstructedMolecule in one go,
they can call TopologyGraph.construct() explicitly.

There is an important design concept this PR highlighted.
The return value of TopologyGraph.construct returns the
ConstructionResult which holds the data necessary to create
a ConstructedMolecule, but also additional data which does not
belong on the ConstructedMolecule, PeriodicInfo is an
example of that. ConstructionResult can be subclassed, as
in PeriodicConstructionResult, if the additional data is specific
to some subset of TopologyGraphclasses. The importance of
having both ConstructedMolecule and ConstructionResult is
that you can do very general things with a ConstructedMolecule,
such as serialization, which you cannot do with a
ConstructionResult. However, ConstructionResult may hold
specific data which you cannot find on a ConstructedMolecule.
So one is generic while the other is specific, and this means each
has different use cases. By allowing the conversion from
ConstructionResult to ConstructedMolecule, the user can use
whatever fits their use case.

The ConstructionState, ConstructionResult and GraphState
classes have the method get_num_building_block() and
get_building_blocks() added. This was necessary so that
ConstructedMolecule.init_from_construction_result() can be
initialized from ConstructionResult only, rather than both
ConstructionResult and TopologyGraph. The
ConstructedMolecule needs to create the _num_building_blocks
attribute, for which it needs to call get_num_building_block()
and get_building_blocks(). Previously these were found on the
TopologyGraph only, but now I'm moving them into the
ConstructionResult too. Having to initialize from both a
ConstructionResult and TopologyGraph is bad because
there is no guarantee that the ConstructionResult was produced
by the TopologyGraph you pass in as the argument.

I considered replacing get_building_blocks() and
get_num_building_block(), with a single method,
get_num_building_blocks(), which would return a dict which maps
each building block to the number of times it appears in the
ConstructedMolecule. However, I decided against this because
the API get_building_block() and get_num_building_block already
exists on TopologyGraph and ConstructedMolecule, so this keeps
the APIs of ConstructionState, ConstructionResult and GraphState
consistent. In addition, the order of building blocks is important,
and while dict instances are ordered, having a get_building_blocks()
method makes the importance of ordering more clear, and easier to
document, in my opinion.

[lgtm-com]

This pull request introduces 2 alerts when merging afa6da7 into 4dc93dd - view on LGTM.com

new alerts:

• 1 for Unused import
• 1 for 'import *' may pollute namespace

[lgtm-com]

This pull request introduces 2 alerts when merging 97858b2 into 4dc93dd - view on LGTM.com

new alerts:

• 1 for Unused import
• 1 for 'import *' may pollute namespace

[lgtm-com]

This pull request introduces 1 alert when merging 168a360 into 4dc93dd - view on LGTM.com

new alerts:

• 1 for 'import *' may pollute namespace

[andrewtarzia]

I think this is a great example of when an inline comment would be helpful - the developer has no idea, without looking at the test, why this would be an unscaled test case...

[lukasturcani]

Good catch. Yeah these test cases are meant to avoid using optimizers which edit the PeriodicInfo. I think putting it into the docstring would work too, as it's essentially part of the interface. Unless you have a particular reason why you prefer inline for this.

[andrewtarzia]

Amazing work.