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

Valence clean-up #7397

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Valence clean-up #7397

wants to merge 2 commits into from

Conversation

ptosco
Copy link
Contributor

@ptosco ptosco commented Apr 28, 2024

This PR cleans up valences in atomic_data.cpp. In addiition to a few missing -1 valences for some alkali metals (Rb, Cs, Fr), which I believe should be added in analogy to Li, Na and K, there were a number of higher valences for Al (6), Si (6), P, As, Sb, Bi (7) which I believe are incorrect. Al has only valence 3 (not 6), Si has only valence 4 (not 6), P, As, Sb, Bi can have 3 and 5 (not 7).
The incorrect valences lead to non-existing compounds such as AlX6, PF6, AsF6, SbF6, BiF6 to be accepted.
The fact that these elements may form negatively charged complexes which higher valence thanks to their d orbitals can be more appropriately accounted for by modifying the code in Atom.cpp. With these modifications, all existing unit tests pass, while preventing species such as AlX6, PF6, AsF6, SbF6, BiF6 to be accepted.
Two Java tests which were failing with the new code in this PR upon inspection turned out to refer to species which are actually incorrect and should be rejected. I have therefore amended such tests.
Incorrect species were:

  • Aluminum oxalate, which was incorrectly represented as Al(C2O42-)3
    image
    while it should have rather been Al2(C2O42-)3
    image
  • Aluminium acetylacetonate, which was incorrectly represented as
    image
    while it should have been
    image
  • Hexafluorosilicate which was incorrectly represented as SiF6 while it should rather have been SiF62-
  • Propylaluminum cation, which was incorrectly represented as
    image
    while it should have been
    image

@ptosco ptosco added bug changes results Cleanup Code cleanup and refactoring labels Apr 28, 2024
@ptosco ptosco marked this pull request as draft April 29, 2024 07:15
@ptosco ptosco changed the title Valence clean-up WIP - DO NOT REVIEW - Valence clean-up Apr 29, 2024
…mbership Al(IIIb), Si(IVb), P(Vb), As(Vb), Sb(Vb), Bi(Vb)

- address elements with d orbitals (P, As, Sb, Si) which can accommodate bonds supported by lone pairs from halogens and chalcogens with requiring adding extra valences to periodic table
- add test to make sure that PX6-, AlX63-, AsX6-, SbX6-, BiF6- do not raise an AtomValenceException while PX6, AlX6, AsX6, SbX6, BiX6 and peracids of the same elements which cannot exist raise an AtomValenceException
- correct SmilesTests.java and rdkit/Chem/UnitTestSmiles.py where CCC[Al+3] should rather read CCC[Al+2]
- comment out incorrect entries in rdkit/Chem/test_data/NCI_aromat_regress.txt and rdkit/Chem/test_data/NCI_5K_TPSA.csv
- add -1 valence to alkali elements which were missing it (Rb, Cs, Fr)
@ptosco ptosco changed the title WIP - DO NOT REVIEW - Valence clean-up Valence clean-up May 2, 2024
@ptosco ptosco marked this pull request as ready for review May 2, 2024 06:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug changes results Cleanup Code cleanup and refactoring
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant