search

partridgejiang / Kekule.js

A Javascript cheminformatics toolkit.
http://partridgejiang.github.io/Kekule.js
MIT License
250 stars 61 forks source link

More Stereoisomer grading failures #104

Closed MrDarkHorse closed 5 years ago

MrDarkHorse commented 5 years ago

I've been trying to carefully illustrate the different classes of use cases I'm seeing.

screen shot 2019-01-18 at 12 36 22 pm screen shot 2019-01-18 at 12 38 35 pm screen shot 2019-01-18 at 12 39 16 pm screen shot 2019-01-18 at 12 39 39 pm screen shot 2019-01-18 at 12 40 00 pm screen shot 2019-01-18 at 12 40 37 pm
MrDarkHorse commented 5 years ago

All of the above I was able to reproduce on the Structure Comparison demo page: http://partridgejiang.github.io/Kekule.js/demos/demoLauncher.html?id=structCompare

But I get even weirder results if I run the structure compare tool directly from the master branch.

MrDarkHorse commented 5 years ago
screen shot 2019-01-18 at 1 04 35 pm screen shot 2019-01-18 at 1 04 49 pm screen shot 2019-01-18 at 1 03 29 pm screen shot 2019-01-18 at 1 03 42 pm screen shot 2019-01-18 at 1 06 33 pm screen shot 2019-01-18 at 1 06 50 pm
partridgejiang commented 5 years ago

Hi @MrDarkHorse, the demo pages had not been updated for a while, and I have just updated them with the latest js files. You may check again now, :).

MrDarkHorse commented 5 years ago

Okay, I agree that updating the demo page now makes the master branch and the demo page act consistently. Unfortunately, most of these things are still an issue.

screen shot 2019-01-23 at 11 49 30 am screen shot 2019-01-23 at 11 49 49 am screen shot 2019-01-23 at 11 50 26 am screen shot 2019-01-23 at 11 50 47 am screen shot 2019-01-23 at 11 51 00 am screen shot 2019-01-23 at 11 53 28 am screen shot 2019-01-23 at 11 53 50 am screen shot 2019-01-23 at 11 54 31 am screen shot 2019-01-23 at 11 54 50 am
partridgejiang commented 5 years ago

Hi @MrDarkHorse, when two groups are on the same side of double bond (anyway, it is not a proper situation for a double bond), the current comparison approach is something like the following figure:

image

The rotation angles of the two groups to the double bond are compared. In that figure above, the two structures are regarded as same. So be the other test cases.

MrDarkHorse commented 5 years ago

I agree that these are "improper" situations for a double bond. We are trying to use this to help teach people how structures work, so we have to give the user freedom to be wrong so that we can give them feedback.

Situations like these are weird, but also lead to false positives in the compare tool.

partridgejiang commented 5 years ago

Eh... that is to say, the improper double bonds should always be regarded as wrong and be different to proper ones in your requirement, e.g.: image vs. image : Different

image vs. image : Different too

If so, I can soon start to implement this feature by adding an additional flag to comparison process.

MrDarkHorse commented 5 years ago

Yes. In my opinion both of those cases should be considered different. One is a little bit wrong, and one is VERY wrong. Haha. :)

partridgejiang commented 5 years ago

Hi @MrDarkHorse , a new commit has been made, please check the latest dist files. The previous improper cases will be regarded as different structure when comparing molecule structures with an additional option flag:

var isSame = mol1.equalStructure(mol2, {strictStereoBondGeometry: true});
MrDarkHorse commented 5 years ago

Awesome! Taking a look now.

MrDarkHorse commented 5 years ago

Well, I'm not sure if I'm doing something wrong, but this still isn't working for me.

I've tried tracing through the code but I'm just getting lost. I'm definitely running the latest version, though. It's not that your dist files are out of date. I'm running the unminified version.

screen shot 2019-01-29 at 10 29 13 am

partridgejiang commented 5 years ago

Eh... maybe some files are not updated well when branches being merged? I have updated the release files again, and modified the structure comparison demo a little (with an extra options panel where you can toggle strictStereoBondGeometry in the UI now). Things should be OK now.

MrDarkHorse commented 5 years ago

Working now! Turns out, in addition to needing the structureCompare updates to include the change, in my code I was running an additional standardize function and not passing the "options" in, so the strict setting was being lost.