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

Check non-flatness #53

Closed
chmnk opened this issue Feb 28, 2025 · 2 comments · Fixed by #54
Closed

Check non-flatness #53

chmnk opened this issue Feb 28, 2025 · 2 comments · Fixed by #54

Comments

@chmnk
Copy link
Contributor

chmnk commented Feb 28, 2025

Some models predict non-flat sp3 rings as flat aromatic ones, as for some sets of atoms aromatic rings prevail in the training set. Would be nice to have a non-flatness check.

I'd be happy to commit this, but not sure what way would you prefer? Making a new file for non-flatness, making a new function, or just adding an option to choose between <= or >= operators to the check_flatness?

@maabuu
Copy link
Owner

maabuu commented Feb 28, 2025

Great idea. I welcome a PR on this. I would prefer the last option, that is adding an option to the flatness function to reuse it for non-flat systems as shown below. I am unsure about the threshold or the SMARTS here - maybe you have some example SMARTS or systems in mind?

Candidate non-flatness config for rings

  - name: "Ring non-flatness"
    function: "flatness"
    parameters:
      flat_systems: # list atoms which together should lie on plane as SMARTS matches
        nonaromatic_5_membered_rings_sp3: "[C,N&!a]1[C,N&!a][C,N&!a][C,N&!a][C,N&!a]1"
        nonaromatic_6_membered_rings_sp3: "[C,N&!a]1[C,N&!a][C,N&!a][C,N&!a][C,N&!a][C,N&!a]1"
      threshold_flatness: 0.25 # max distance in A to closest shared plane
      check_nonflat: True
    chosen_binary_test_output:
      - flatness_passes
    rename_outputs:
      num_systems_checked: number_aromatic_rings_checked
      num_systems_passed: number_aromatic_rings_pass
      max_distance: aromatic_ring_maximum_distance_from_plane
      flatness_passes: "Aromatic ring flatness"

Existing flatness config for rings

  - name: "Ring flatness"
    function: "flatness"
    parameters:
      flat_systems: # list atoms which together should lie on plane as SMARTS matches
        aromatic_5_membered_rings_sp2: "[ar5^2]1[ar5^2][ar5^2][ar5^2][ar5^2]1"
        aromatic_6_membered_rings_sp2: "[ar6^2]1[ar6^2][ar6^2][ar6^2][ar6^2][ar6^2]1"
      threshold_flatness: 0.25 # max distance in A to closest shared plane
    chosen_binary_test_output:
      - flatness_passes
    rename_outputs:
      num_systems_checked: number_aromatic_rings_checked
      num_systems_passed: number_aromatic_rings_pass
      max_distance: aromatic_ring_maximum_distance_from_plane
      flatness_passes: "Aromatic ring flatness"

@chmnk
Copy link
Contributor Author

chmnk commented Mar 3, 2025

Thank you!

I am unsure about the threshold or the SMARTS here

the default >=0.1 threshold actually looks ok to me

I'd probably go for something like this, allowing for the nonflat systems with double bonds?

"nonaromatic_5_membered_rings": "[C,O,S,N&!a]~1[C,O,S,N&!a][C,O,S,N&!a][C,O,S,N&!a][C,O,S,N&!a]1",
"nonaromatic_6_membered_rings": "[C,O,S,N&!a]~1[C,O,S,N&!a][C,O,S,N&!a][C,O,S,N&!a][C,O,S,N&!a][C,O,S,N&!a]1",

I was also thinking about these:

"nonaromatic_6_membered_rings_db03": "[C,O,S,N&!a]~1[C,O,S,N&!a][C,O,S,N&!a]~[C,O,S,N&!a][C,O,S,N&!a][C,O,S,N&!a]1",
"nonaromatic_6_membered_rings_db02": "[C,O,S,N&!a]~1[C,O,S,N&!a][C,O,S,N&!a][C,O,S,N&!a]~[C,O,S,N&!a][C,O,S,N&!a]1",

but 'C1C=CN=CN1' turns out to be flat.

@chmnk chmnk mentioned this issue Mar 3, 2025
@maabuu maabuu linked a pull request Mar 4, 2025 that will close this issue
@maabuu maabuu closed this as completed in #54 Mar 4, 2025
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

Successfully merging a pull request may close this issue.

2 participants