You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I have found that this is not only a problem for what is discussed in the commit but also after a merging of structures. For example:
Template (S1) has the substructure C2=C2-C3 with atom indices of [1, 2, 3]
Structure to merge (S2) is a ring: C2=C2-C3-C3-C3-C3-1 with atom indices [1, 4, 5, 8, 11, 14] (plus hydrogens)
If S2 has a RCA4 between atoms [14, 1, 4, 5] and is then merged onto the template the resulting RCA4 bond property of the merged structure (S3) will be listed as [14, 1, 2, 5], but atoms 14 and 15 are not S2[14] an S2[15] anymore because of the merging sequence.
It might be better to read/write the atom index, but when manipulating structures this property becomes an schrodinger.structure._structureatom object.
The text was updated successfully, but these errors were encountered:
Agreed that using atom objects would be ideal, but I ran into some problems when I tried to. That was my original implementation, but I couldn't get it to work. Here's a rough example that demonstrates the problem. The syntax isn't exact since I'm writing this off the top of my head.
s1 = schrodinger.structure # Say it has 5 atoms
s2 = schrodinger.structure # Say it also has 5 atoms
s1_a4 = s1.atom[4] # Can't remember, but this may just be an iterator
s1_a4.index
# Returns 4
s2_a3 = s2.atom[3]
s2_a3.index
# Returns 3
s12 = s1.merge(s2)
s1_a4.index
# Returns 4
s12_a4 = s12.atom[4]
s12_a4.index
# Returns 4
s1_a4 == s12_a4
# Returns False
s2_a3.index
# Returns 3
# We would want that to be 8, but it's not
s12_a8 = s12.atom[8]
s12_a8.index
# Returns 8
s12_a4 == s12_a8
# Returns False
# It gets worse
s1_a1 = s1.atom[1]
s1_a1 == st.atom[1]
# Returns False
Basically, every time you iterate over a structure's atoms, it generates the atom objects for you. The atom objects aren't stored, so there's no way to hold onto them.
I tried this sort of implementation for about a week before giving up and going to the simple integers, but maybe you can come up with a way to make it work. It would make way more sense if we could do it this way, and it would make everything so much simpler. For example, we could probably remove crap like this.
Since the argparse argument was already present without any use, I added
in logic to gather data based off of the users preference with the default
being 'OPT'. This is useful if you want to gather data from a particular
substructure that you are not parameterizing.
This comment thread provides the initial concerns with the atom index as the bond property for TORC and RCA4.
https://github.com/ericchansen/q2mm/commit/2e9c2e98b97a0603961973ec08de487ce687d922#commitcomment-21991092
I have found that this is not only a problem for what is discussed in the commit but also after a merging of structures. For example:
Template (S1) has the substructure C2=C2-C3 with atom indices of [1, 2, 3]
Structure to merge (S2) is a ring: C2=C2-C3-C3-C3-C3-1 with atom indices [1, 4, 5, 8, 11, 14] (plus hydrogens)
If S2 has a RCA4 between atoms [14, 1, 4, 5] and is then merged onto the template the resulting RCA4 bond property of the merged structure (S3) will be listed as [14, 1, 2, 5], but atoms 14 and 15 are not S2[14] an S2[15] anymore because of the merging sequence.
It might be better to read/write the atom index, but when manipulating structures this property becomes an schrodinger.structure._structureatom object.
The text was updated successfully, but these errors were encountered: