-
Notifications
You must be signed in to change notification settings - Fork 10
Description
This is a list of my planned fix for the problem in PR #68. First, It appears that the test that is failing does in fact have a more complex set of molecules so it makes a little more sense why one is failing. Essentially what this comes down to though is what happens when there is a tie in the scoring function in the find_pair function in the cluster_graph code. I had written SMIRKS based on my understanding where that function would pair atoms to storage places in the order they were given when there were ties. The problem in PR #68 appears to be that in 3.5 this was the case, but there was a change in networkx that leads to the ties being treated slightly differently.
Ultimately this shouldn't be behavior that depends on networkx. I think it would be preferable if the score was updated based on neighboring atoms when there is a tie on the current atoms...
I'm still thinking about how to generalize this, what if you really have two identical molecules or very symmetric molecules where there is a REAL tie in the score? I will add more detail about how I found the problem in a little bit, but I wanted to do an initial brain dump.
@janash I'm getting closer to narrowing down the issue.