kjappelbaum / moffragmentor

Split a MOF into its building blocks.
https://moffragmentor.readthedocs.io/en/latest/
MIT License
21 stars 0 forks source link

Breaks with networkx 3.4 #166

Open johannbrehmer opened 1 week ago

johannbrehmer commented 1 week ago

Yet another great tool from you!

It works fine with me with networkx < 3.4.0, but it breaks with the latest versions of networkx.

When I call

mof = MOF.from_cif(path_to_any_cif_file)
fragments = mof.fragment()

I get this:

    fragments = mof.fragment()
                │   └ <function MOF.fragment at 0x33fb30220>
                └ <moffragmentor.mof.MOF object at 0x478250ed0>

  File "/Users/johann/micromamba/envs/mofgen/lib/python3.11/site-packages/moffragmentor/mof.py", line 316, in fragment
    return self._fragment(
           │    └ <function MOF._fragment at 0x33fb30180>
           └ <moffragmentor.mof.MOF object at 0x478250ed0>
  File "/Users/johann/micromamba/envs/mofgen/lib/python3.11/site-packages/moffragmentor/mof.py", line 285, in _fragment
    fragmentation_result = run_fragmentation(
                           └ <function run_fragmentation at 0x33fb16d40>
> File "/Users/johann/micromamba/envs/mofgen/lib/python3.11/site-packages/moffragmentor/fragmentor/__init__.py", line 67, in run_fragmentation
    linker_collection = create_linker_collection(
                        └ <function create_linker_collection at 0x33faffb00>
  File "/Users/johann/micromamba/envs/mofgen/lib/python3.11/site-packages/moffragmentor/fragmentor/linkerlocator.py", line 141, in create_linker_collection
    linker_collection = _create_linkers_from_node_location_result(
                        └ <function _create_linkers_from_node_location_result at 0x33faff880>
  File "/Users/johann/micromamba/envs/mofgen/lib/python3.11/site-packages/moffragmentor/fragmentor/linkerlocator.py", line 124, in _create_linkers_from_node_location_result
    if linker.hash not in found_hashes and frac_center not in found_frac_centers:
       │      │           │                │                  └ set()
       │      │           │                └ (0.0392, 0.9856, 0.0109)
       │      │           └ set()
       │      └ <functools.cached_property object at 0x32cd951d0>
       └ Linker (C10 H4 O8)
  File "/Users/johann/micromamba/envs/mofgen/lib/python3.11/functools.py", line 1001, in __get__
    val = self.func(instance)
          │    │    └ Linker (C10 H4 O8)
          │    └ <function SBU.hash at 0x33faf3600>
          └ <functools.cached_property object at 0x32cd951d0>
  File "/Users/johann/micromamba/envs/mofgen/lib/python3.11/site-packages/moffragmentor/sbu/sbu.py", line 288, in hash
    wl_hash = self.weisfeiler_lehman_graph_hash
              │    └ <functools.cached_property object at 0x32cd95290>
              └ Linker (C10 H4 O8)
  File "/Users/johann/micromamba/envs/mofgen/lib/python3.11/functools.py", line 1001, in __get__
    val = self.func(instance)
          │    │    └ Linker (C10 H4 O8)
          │    └ <function SBU.weisfeiler_lehman_graph_hash at 0x33faf34c0>
          └ <functools.cached_property object at 0x32cd95290>
  File "/Users/johann/micromamba/envs/mofgen/lib/python3.11/site-packages/moffragmentor/sbu/sbu.py", line 273, in weisfeiler_lehman_graph_hash
    return nx.weisfeiler_lehman_graph_hash(self.molecule_graph.graph, node_attr="specie")
           │  │                            │    │              └ <networkx.classes.multidigraph.MultiDiGraph object at 0x47891be50>
           │  │                            │    └ Molecule Graph
           │  │                            │      Molecule: 
           │  │                            │      Molecule Summary
           │  │                            │      Site: C (13.9081, 10.6990, 95.0739)
           │  │                            │      Site: C (12.9586, 11.1683, 96.1193)
           │  │                            │      Site: H (1...
           │  │                            └ Linker (C10 H4 O8)
           │  └ <function weisfeiler_lehman_graph_hash at 0x32a319bc0>
           └ <module 'networkx' from '/Users/johann/micromamba/envs/mofgen/lib/python3.11/site-packages/networkx/__init__.py'>
  File "<class 'networkx.utils.decorators.argmap'> compilation 94", line 3, in argmap_weisfeiler_lehman_graph_hash_90
  File "/Users/johann/micromamba/envs/mofgen/lib/python3.11/site-packages/networkx/utils/decorators.py", line 91, in _not_implemented_for
    raise nx.NetworkXNotImplemented(errmsg)
          │  │                      └ 'not implemented for multigraph type'
          │  └ <class 'networkx.exception.NetworkXNotImplemented'>
          └ <module 'networkx' from '/Users/johann/micromamba/envs/mofgen/lib/python3.11/site-packages/networkx/__init__.py'>

networkx.exception.NetworkXNotImplemented: not implemented for multigraph type

I believe this is because networkx 3.4.0 added the not_implemented_for("multigraph") decorator to weisfeiler_lehman_graph_hash(), see the changelog.

kjappelbaum commented 3 days ago

I do not know if I'd call those great tools - especially if I do not manage to find time to maintain them properly. Thanks for spotting and reporting!

Should probably also be fixed in structuregraph-helpers.

Using the networkx implementation for the hash was always suboptimal. Perhaps we should implement our own version.