snurr-group / mofid

A system for rapid identification and analysis of metal-organic frameworks
https://snurr-group.github.io/mofid/
GNU General Public License v2.0
42 stars 24 forks source link

Complete MOFid with extraneous error flag #27

Closed Andrew-S-Rosen closed 3 years ago

Andrew-S-Rosen commented 3 years ago

For this this mof, the generated MOFid is [Co][O]([Co])([Co])[Co].[O-]C(=O)N=NC(=O)[O-].[O-]C(=O)[N][N]C(=O)[O-] MOFid-v1.bcu,ERROR.cat0;199. All the components of the MOFid are present and seemingly valid, but with a ",ERROR" that should not be there.

bbucior commented 3 years ago

Fortunately, I think resolving this issue should be pretty fast, depending on what output users would expect. The current Python code reports both the single-node vs. all-node topologies to be explicit, but it does not have to stay that way.

I'll keep the changes as comments here instead of a PR for you, in case I'm misunderstanding the request or there's other edge cases. For the use case you provide, one of the Python scripts could be updated to ignore a second "ERROR" topology:

if sn_topology == an_topology or an_topology == 'ERROR':

and if required, also the website logic can be sync'd (and since it's a small patch, the live website can be updated directly):

} else if (single_node_topology == all_node_topology || all_node_topology == 'ERROR') {

Andrew-S-Rosen commented 3 years ago

Hi @bbucior! Thanks for dropping by with your thoughts there (that moment when Ben drops by unexpectedly :D). After reading a bit more, I came to the same conclusion as well (I just needed to find the write spot in the code -- thanks for doing that for me!). I'll test this out a bit before doing an update. I think it would be a pretty reasonable adjustment.