Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Force Overlap Results in Missing Adjacencies #1048

Closed
CalCraven opened this issue Aug 2, 2022 · 4 comments
Closed

Force Overlap Results in Missing Adjacencies #1048

CalCraven opened this issue Aug 2, 2022 · 4 comments

Comments

@CalCraven
Copy link
Contributor

Bug summary

What were you trying to do and what happened instead? Please copy and paste the stack output
When converting to GMSO, the bond graph adjacencies are necessary to evaluate the definitions of molecules.

Code to reproduce the behavior

Please include a code snippet that can be used to reproduce this bug.

import mbuild as mb

def print_adjacencies(cpd):
    bgraph = cpd.bond_graph
    for node in bgraph.nodes():
        x = map(lambda node: node.name, bgraph._adj[node])
        print(f"Node {node.name}, which is bonded to {list(x)}")

def get_missing_edges(compound):
    bgraph = compound.bond_graph
    missing_edges = []
    for edge0, edge1 in bgraph.edges():
        try:
            assert bgraph.has_edge(edge0, edge1)
            assert bgraph.has_edge(edge1, edge0)
        except AssertionError:
            missing_edges.append((edge0, edge1))
    print(len(missing_edges))
    return missing_edges

# Test for working version
ch3 = mb.lib.moieties.CH3()
ch3_2 = mb.lib.moieties.CH3()
ch3.add_bond((ch3,ch3_2))
ch3.add(ch3_2)
get_missing_edges(ch3)

# Test for overlaps version
ch3 = mb.lib.moieties.CH3()
ch3_2 = mb.lib.moieties.CH3()
mb.force_overlap(ch3_2, ch3_2["up"], ch3["up"])
ch3.add(ch3_2)
get_missing_edges(ch3)

Output

0
3
[(<H pos=([-1.0700e-01 -1.4000e-01 -6.2735e-18]), 1 bonds, id: 140628872568400>,
  <C pos=([ 2.8197e-17 -1.4000e-01 -1.4644e-17]), 1 bonds, id: 140628872581952>),
 (<H pos=([ 0.0357 -0.2169  0.0653]), 1 bonds, id: 140628871527632>,
  <C pos=([ 2.8197e-17 -1.4000e-01 -1.4644e-17]), 1 bonds, id: 140628872581952>),
 (<H pos=([ 0.0357 -0.1581 -0.0993]), 1 bonds, id: 140628871527536>,
  <C pos=([ 2.8197e-17 -1.4000e-01 -1.4644e-17]), 1 bonds, id: 140628872581952>)]

Software versions

  • Which version of mBuild are you using? (python -c "import mbuild as mb; print(mb.__version__)")
    0.15.0
  • Which version of Python (python --version)?
    3.8
  • Which operating system?
    MacOS
@CalCraven
Copy link
Contributor Author

If you use print adjacencies on the defunct CH3 class, you get

Node C, which is bonded to ['H', 'H', 'H', 'C']
Node H, which is bonded to ['C']
Node H, which is bonded to ['C']
Node H, which is bonded to ['C']
Node C, which is bonded to ['C']
Node H, which is bonded to ['C']
Node H, which is bonded to ['C']
Node H, which is bonded to ['C']

Whereas the correctly made one looks like:

Node C, which is bonded to ['H', 'H', 'H']
Node H, which is bonded to ['C']
Node H, which is bonded to ['C']
Node H, which is bonded to ['C']
Node CH3, which is bonded to []
Node C, which is bonded to ['H', 'H', 'H']
Node H, which is bonded to ['C']
Node H, which is bonded to ['C']
Node H, which is bonded to ['C']

@CalCraven
Copy link
Contributor Author

This also results in an incorrect bondgraph, so it is in the add_bond method.

ch3 = mb.lib.moieties.CH3()
ch3_2 = mb.lib.moieties.CH3()
ch3.add_bond((ch3[0], ch3_2[0]))
ch3.add(ch3_2)
get_missing_edges(ch3)
print_adjacencies(ch3)
3
Node C, which is bonded to ['H', 'H', 'H', 'C']
Node H, which is bonded to ['C']
Node H, which is bonded to ['C']
Node H, which is bonded to ['C']
Node C, which is bonded to ['C']
Node H, which is bonded to ['C']
Node H, which is bonded to ['C']
Node H, which is bonded to ['C']

@CalCraven
Copy link
Contributor Author

Okay so here looks to be the issue. When adding a bond, using the add_bond method, the node is added to the bond graph via this line

self.root.bond_graph.add_edge(particle_pair[0], particle_pair[1])

Then, when the self.add() method is called to add that moiety to the compound, only the atom that has had the bond added to it is part of the main compound bond graph. Following this line in the compose() in bond_graph.py:

(adj[node].add(neighbor) for neighbor in neighbors)

This should add the other members of the original bond_graph to the set. However, this doesn't work in one line for loops.

x = [1,2,3,4,5,1]
y = set()
(y.add(val) for val in x)
print("Try 1:", y)
x = [1,2,3,4,5,1]
y = set()
[y.add(val) for val in x]
print("Try 2: ", y)

Output:

Try 1: set()
Try 2:  {1, 2, 3, 4, 5}

@daico007
Copy link
Member

daico007 commented Aug 8, 2022

Fixed by #1049

@daico007 daico007 closed this as completed Aug 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants