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

Fix energy ordering of electronic configurations for Elements and Species #3851

Closed
wants to merge 15 commits into from

Conversation

rkingsbury
Copy link
Contributor

@rkingsbury rkingsbury commented May 30, 2024

Summary

  • Rearranges electronic structures according to the correct energy levels. Checked against
  • implements full_electronic_structure, electronic_structure, and valence for Species
  • "prettify" periodic_table.json to make it more readable as a standalone file

https://ptable.com/#Electrons/Expanded
and
https://chem.libretexts.org/Bookshelves/Physical_and_Theoretical_Chemistry_Textbook_Maps/Supplemental_Modules_(Physical_and_Theoretical_Chemistry)/Quantum_Mechanics/10%3A_Multi-electron_Atoms/Electron_Configuration

Fixes #3850
Fixes #3849

@rkingsbury
Copy link
Contributor Author

Note: due to 'prettifying' the .json file, the full diff is not very useful. I suggest viewing the changes associated only with the 2nd commit and/or using https://jsondiff.com/ to see exactly which values I changed.

@rkingsbury rkingsbury changed the title Fix energy ordering of electronic configurations for Elements Fix energy ordering of electronic configurations for Elements and Species May 30, 2024
@rkingsbury
Copy link
Contributor Author

rkingsbury commented May 30, 2024

This change seems to have caused some problems with get_crystal_field_spin, causing a few test failures. Can anyone offer insight into how this should be modified (I'm not familiar with this method)

         # taken from get_crystal_field_spin
        elec = species.element.full_electronic_structure
        if len(elec) < 4 or elec[-1][1] != "s" or elec[-2][1] != "d":
>           raise AttributeError(f"Invalid element {species.symbol} for crystal field calculation.")
E           AttributeError: Invalid element Ti for crystal field calculation.

EDIT: OK, I think I corrected all the tests that were affected by ordering of orbitals.

@mkhorton
Copy link
Member

mkhorton commented Jun 1, 2024

Thanks for fixing @rkingsbury, happy to merge this but could you add a reference to the docstring? If it was something peer-reviewed even better. Noting this NIST reference too via https://en.wikipedia.org/wiki/Electron_configurations_of_the_elements_(data_page)

Separate to this PR, I wonder if the periodic table file might be better stored as yaml so we can include in-line comments.

@janosh
Copy link
Member

janosh commented Jun 3, 2024

might be better stored as yaml so we can include in-line comments

yes please! that would be great

@rkingsbury
Copy link
Contributor Author

Thanks for fixing @rkingsbury, happy to merge this but could you add a reference to the docstring? If it was something peer-reviewed even better. Noting this NIST reference too via https://en.wikipedia.org/wiki/Electron_configurations_of_the_elements_(data_page)

Will do! I will add this NIST reference as well, which also includes first cations. There seem to be a small number of cases where the ion configuration is NOT the same as that of the next lowest element. I'll investigate and either modify the docstring of the Species methods and/or add logic to catch those cases.

@rkingsbury
Copy link
Contributor Author

rkingsbury commented Jun 4, 2024

The plot thickens

@mkhorton I've added a reference to the NIST resource you found, which is among the best available (I think) and includes a well formatted citation. However, reviewing this data raised two points that may complicate this PR:

  1. In the NIST data, electron configuration is not ordered by energy level. For example, the electron config for Fe is listed as [Ar]3d64s2 even though the 4s2 electrons have lower energy. Personally I think we should list them in order of increasing energy level in pymatgen because 1) the string then contains more information and 2) many other sources (such as, ptable.com) do list them sorted by energy level, and 3) it helps guide intuition about which electron(s) may ionize first. However, it's fair to discuss this point.

  2. The NIST data also has electron configurations for ions. This is great, but it showed that my initial assumption in, e.g. Species.get_electronic_structure to return the electron configuration of an isoelectronic atom was not a good one. Although it works OK for s and p orbitals, when d electrons are involved it appears that ions lose their s electrons before d. For example, the correct configuration for Fe+3 is [Ar]3_d_5, while the configuration of isoelectronic V(0) is [Ar]3_d_5

Point 2) begs the question of how best to handle this within pymatgen. The best (but labor-intensive) solution would be to:

  • add the ionized electronic configurations to periodic_table.json for at least all of the common oxidation states of every element.
  • Fall back to the isoelectronic treatment and issue a warning for other oxidation states

This work should perhaps be a separate PR, in which case I can revert to the NotImplementedError in this PR. What do you think?

@mkhorton
Copy link
Member

mkhorton commented Jun 9, 2024

I don't have strong opinions on 1. provided that it's documented.

For 2., I would always prefer we give accurate and referenced information when possible. In several places, pymatgen currently provides a "best guess", which might be helpful, but can mislead. Especially as pymatgen is now quite a widely-used and mature code, I am getting more concerned about these cases. At minimum, we should warn.

For this PR, I am happy to proceed with your best judgement.

@shyuep
Copy link
Member

shyuep commented Jun 11, 2024

@rkingsbury I suggest (a) follow the NIST convention in returning the electronic structure for the elements. I would note that that this is the reason why the existing electronic structure is defined the way it is in any case. (b) Clarify the docs that the electronic structure is based on the element, whether for Element or Species, (c) add a separate ion_electronic_structure For species, add an ion_electronic_structure or species_electronic_structure that returns the actual electronic structure of the species based on energy levels. I think this is the most unlikely to cause problems in downstream codes, even though I don't expect that electronic_structure is used much.

@rkingsbury
Copy link
Contributor Author

@rkingsbury I suggest (a) follow the NIST convention in returning the electronic structure for the elements. I would note that that this is the reason why the existing electronic structure is defined the way it is in any case.

OK, agreed. After some further research, both NIST and the CRC handbook list the electronic configurations that way (i.e., NOT reflecting Madelung's rule). I was definitely taught to write them the other way, but this appears not to be the "best practice" anymore (or maybe it never was). So we don't need to change the order.

(b) Clarify the docs that the electronic structure is based on the element, whether for Element or Species (c) add a separate ion_electronic_structure For species, add an ion_electronic_structure or species_electronic_structure that returns the actual electronic structure of the species based on energy levels. I think this is the most unlikely to cause problems in downstream codes, even though I don't expect that electronic_structure is used much.

Do you mean that Species(Fe, 3).electronic_structure would still return the electron config of Fe(0) whereas you would need to call Species(Fe,3).ion_electronic_structure to get the actual config of the ion?

I'm concerned about potential for confusion / misinterpretation if we take that approach, as opposed to just using electronic_structure for both Element and Species (but using ion-specific data for Species). Are you concerned that downstream codes are already calling electronic_structure on Species, and hence might break if we implement it? Because if anyone is doing that, they should now be getting a NotImplementedError.

Whichever way we land on that, I think the best course of action is to close this PR without merging, and I'll open a new one with ion-specific electron config data.

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 this pull request may close these issues.

Species.full_electronic_structure is incorrect electronic_structure energy ordering is incorrect
4 participants