dwave-networkx icon indicating copy to clipboard operation
dwave-networkx copied to clipboard

add topology enums and dispatch decorator

Open boothby opened this issue 1 year ago • 2 comments

This is a preliminary patch to address #210 -- we add a topology enum to dnx.topology, and a decorator topology_dispatch.

The topology enum is pretty simple, except that I want the machinery of py3.11's StrEnum (that is, CHIMERA == 'chimera'), so I fair-use'd barebones implementations of StrEnum and global_enum from the CPython3.11 library.

The topology_dispatch decorator can be used to define a generic function in one file, and then in other files, define specializations based on the topology family. For example, I've added a defect_free generic function in dnx.generators.common and then added specializations for each of CHIMERA, PEGASUS, ZEPHYR. Thus, to get a defect-free version of any qubit topology, one can simply call dnx.generators.common.defect_free(G).

#############
# common.py
#
@topology_dispatch
def defect_free(G):
    """Construct a defect-free topology based on the properties of G."""
    raise NotImplementedError(f"no defect-free generator defined for {G.graph.get('family')} graphs")


#############
# chimera.py
#
from ..topology import CHIMERA
from .common import defect_free

@defect_free.install_dispatch(CHIMERA)
def defect_free_chimera(G):
    """Construct a defect-free Chimera graph based on the properties of G."""
    attrib = G.graph
    family = attrib.get('family')
    if family != CHIMERA:
        raise ValueError("G must be constructed by dwave_networkx.chimera_graph")
    args = attrib['rows'], attrib['columns'], attrib['tile']
    kwargs = {'coordinates': attrib['labels'] == 'coordinate'}
    return chimera_graph(*args, **kwargs)

boothby avatar Dec 25 '24 00:12 boothby

Codecov Report

Attention: Patch coverage is 90.47619% with 12 lines in your changes missing coverage. Please review.

Project coverage is 76.68%. Comparing base (7d8aa45) to head (e8bd004).

Files with missing lines Patch % Lines
dwave_networkx/drawing/pegasus_layout.py 66.66% 3 Missing :warning:
dwave_networkx/drawing/chimera_layout.py 75.00% 2 Missing :warning:
dwave_networkx/drawing/zephyr_layout.py 75.00% 2 Missing :warning:
dwave_networkx/topology.py 93.75% 2 Missing :warning:
dwave_networkx/generators/chimera.py 94.11% 1 Missing :warning:
dwave_networkx/generators/pegasus.py 95.23% 1 Missing :warning:
dwave_networkx/generators/zephyr.py 95.00% 1 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #248      +/-   ##
==========================================
+ Coverage   75.05%   76.68%   +1.62%     
==========================================
  Files          31       32       +1     
  Lines        2213     2290      +77     
==========================================
+ Hits         1661     1756      +95     
+ Misses        552      534      -18     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Dec 25 '24 00:12 codecov[bot]

After some thought, I wasn't a fan of my first approach; it was a bit too kludgy. Re-reading the comments of dwavesystems/minorminer#210, @pau557 had given me a better suggestion for notation which I hadn't followed in the above. My revised approach collects generic methods into the enumeration members. Here, I avoid circular imports through the use of an ImplementationHook which is used to monkeypatch itself away during the initial import.

With this new approach, I don't need to add generic functions to multiple files; instead, I collect their names into topology.py. Instead of that defect_free function, I instead make a defect_free_graph slot for the Topology enum members, and when I implement defect_free_chimera, it looks like:

@CHIMERA.defect_free_graph.implementation
def defect_free_chimera(G):
    ...

After making this change, my choice to use StrEnum had a negative impact: the namespace was polluted with the various methods of str. On reflection, I think that it's important to preserve a few features of str since I'm populating the dwave_networkx graph "family" property with these enum members -- namely, __eq__, __hash__, and __str__ should be enough for backwards compatibility and future convenience.

boothby avatar Dec 31 '24 23:12 boothby