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

StructureGroupBuilder and StructureGroupDoc minor fixes #494

Merged
merged 3 commits into from Aug 8, 2022

Conversation

kim-jiyoon
Copy link
Contributor

There are 2 small bugs found and things should now be working as expected after testing the StructureGroupBuilder on the MP materials collection and not just my personal materials collection.

  1. The field name was not updated for the "host_material_ids" since a small change was made to the field
  2. The StructureGroupBuilder and StructureGroupDoc originally did not consider energy data, but now that it is being used to determine the lowest energy "host_material_id" when there are multiple potential ids to choose from, the existing method of generating a ComputedStructureEntry in the " _entry_from_mat_doc" method needed to be revised. This is a more robust method because it uses all the information available in the MaterialsDoc's entries field and checks for the run type, which is not always just GGA or GGA+U especially in the MP materials collection. The MP compatibility scheme can only apply to calculations performed with GGA or GGA+U, and GGA+U is prioritized because it is a higher level of theory.

@munrojm @acrutt

@codecov-commenter
Copy link

codecov-commenter commented Aug 5, 2022

Codecov Report

Merging #494 (3b6bef6) into main (662e393) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main     #494   +/-   ##
=======================================
  Coverage   97.56%   97.56%           
=======================================
  Files         116      116           
  Lines       24075    24075           
=======================================
  Hits        23488    23488           
  Misses        587      587           
Impacted Files Coverage Δ
emmet-core/emmet/core/structure_group.py 93.27% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@munrojm munrojm added the release:patch Patch updates label Aug 8, 2022
@munrojm
Copy link
Member

munrojm commented Aug 8, 2022

Hi @kim-jiyoon, this looks good to me! Thank you for the fixes.

@munrojm munrojm merged commit aeb8a8c into materialsproject:main Aug 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:patch Patch updates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants